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

Fix Plaid modal by updating to latest version of react-native-plaid-link-sdk / resolve stashed token issue #4962

Merged
merged 5 commits into from
Sep 2, 2021

Conversation

marcaaron
Copy link
Contributor

@marcaaron marcaaron commented Aug 31, 2021

Details

cc @Julesssss @luacmartins

Fixed Issues

$ #4959

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

Mobile Web

Desktop

iOS

2021-08-31_13-15-37
2021-08-31_13-15-47

Android

@marcaaron marcaaron self-assigned this Aug 31, 2021
@marcaaron marcaaron requested a review from a team as a code owner August 31, 2021 22:29
@MelvinBot MelvinBot requested review from yuwenmemon and removed request for a team August 31, 2021 22:29
CAPTURE_METRICS: lodashGet(Config, 'CAPTURE_METRICS', false),
ONYX_METRICS: lodashGet(Config, 'ONYX_METRICS', false),
CAPTURE_METRICS: lodashGet(Config, 'CAPTURE_METRICS', 'false') === 'true',
ONYX_METRICS: lodashGet(Config, 'ONYX_METRICS', 'false') === 'true',
Copy link
Contributor Author

@marcaaron marcaaron Aug 31, 2021

Choose a reason for hiding this comment

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

Note: These changes are not related, but I noticed my test builds were showing the metrics despite being set to CAPTURE_METRICS=false in .env file and decided to fix this here.

@marcaaron marcaaron changed the title [WIP] Fix Plaid modal by updating to latest version of react-native-plaid-link-sdk Fix Plaid modal by updating to latest version of react-native-plaid-link-sdk Aug 31, 2021
@marcaaron
Copy link
Contributor Author

This should be ready for review now

@marcaaron
Copy link
Contributor Author

So... actually.... I'm still experiencing issues on Android on debug builds. Building for release works fine. @yuwenmemon lmk if you need help figuring out how to do this (there is a SO post somewhere).

@marcaaron
Copy link
Contributor Author

Hmm actually it's worse than I thought and also happens in release but inconsistently.
Gonna put this back into draft and file see if I can get some help from Plaid.

@marcaaron marcaaron changed the title Fix Plaid modal by updating to latest version of react-native-plaid-link-sdk Fix Plaid modal by updating to latest version of react-native-plaid-link-sdk / resolve stashed token issue Sep 1, 2021
@marcaaron
Copy link
Contributor Author

Ok, this is ready for a review now.

There is still an issue with Plaid that causes the UI to load very slowly and basically makes it seem broken on dev, but I got to the bottom of the worst of the Android issues. Plaid is aware of the issue and working on it - it's related to their web bundle that is getting served via a WebView. But not sure what we can do about this from our end at the moment.

A more extreme idea is to create our own native module, but Plaid is also working on that for react-native so it seems like we should #do-nothing and wait for improvements.

@marcaaron marcaaron marked this pull request as ready for review September 1, 2021 22:10
Copy link
Contributor

@luacmartins luacmartins left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @marcaaron! The Plaid modal is opening on Android now! I still see the following error when confirming the account though. Is that part of the issue that Plaid is trying to fix?

Invariant Violation: No callback found with cbID 17868 and callID 8934 for  PlaidAndroid.startLinkActivityForResult - most likely the callback was already invoked. Args: '[{"publicToken":"public-sandbox-53eb79a5-bfdd-4aa5-80c7-e45fa0695ed0","metadata":{"metadataJson":"{\"institution\":{\"name\":\"Chase\",\"institution_id\":\"ins_3\"},\"account\":{\"id\":null,\"name\":null,\"type\":nul...(truncated)...","linkSessionId":"0f3901e3-83f7-48b3-8191-e12c818c3247","institution":{"name":"Chase","id":"ins_3"},"accounts":[{"subtype":"checking","type":"depository","mask":"0000","name":"Plaid Checking","id":"jq958bm9a6s5Errllw8AtW3pPd35lWFy9ez3n"},{"subtype":"savings","type":"depository","mask":"1111","name":"Plaid Saving","id":"7Mxb9p5xeNFZ3zzjjDwEhxvDRVv1gxCN3qAGv"}]}}]', js engine: hermes

@marcaaron
Copy link
Contributor Author

marcaaron commented Sep 2, 2021 via email

@luacmartins
Copy link
Contributor

I'll try testing again tomorrow!

Copy link
Contributor

@Julesssss Julesssss left a comment

Choose a reason for hiding this comment

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

Working flawlessly for me on Android devices, thanks for looking into this. Personally, I've never found the slow loading time to be that problematic, but nice to hear Plaid is looking into improvements anyway.

@luacmartins
Copy link
Contributor

Just tested again and it's working!

@luacmartins luacmartins merged commit 163746e into main Sep 2, 2021
@luacmartins luacmartins deleted the marcaaron-fixPlaid branch September 2, 2021 16:06
@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.

@marcaaron
Copy link
Contributor Author

Personally, I've never found the slow loading time to be that problematic, but nice to hear Plaid is looking into improvements anyway.

Oh nice! It could be related to my device or internet now that I think about it. Glad to hear it's only slow for me!

@marcaaron
Copy link
Contributor Author

@Julesssss @luacmartins thanks for testing! 🙇

@OSBotify
Copy link
Contributor

OSBotify commented Sep 3, 2021

🚀 Deployed to staging by @luacmartins 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 @luacmartins Hello! We're currently unable to test this in iOS and Android since we can't navigate to a specific URL in-app This might have to be tested internally or if you know another method please let us know.

@marcaaron
Copy link
Contributor Author

Same answer as here -> #5029 (comment) but please let us know here if that's not going to work! Thanks!

@isagoico
Copy link

isagoico commented Sep 3, 2021

This issue is failing this PR #5075

@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.

5 participants