-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[HOLD for payment 2023-11-07] [$500] iOS: "Connect online with Plaid" popup is opened repeatedly after resuming the app. #27616
Comments
Triggered auto assignment to @CortneyOfstad ( |
Job added to Upwork: https://www.upwork.com/jobs/~01b7bfe22feb2b9545 |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to @adelekennedy ( |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ntdiary ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.iOS: "Connect online with Plaid" popup is opened repeatedly after resuming the app What is the root cause of that problem?Problem is App/src/components/PlaidLink/index.native.js Lines 20 to 28 in c77814f
and
resulting unmounting of What changes do you think we should make in order to solve the problem?We can update const ref = useRef({ isVisible: false })
.....
.....
useEffect(() => {
ref.current.isVisible = true
openLink({
.....
onSuccess: ({ publicToken, metadata }) => {
ref.current.isVisible = false;
props.onSuccess({ publicToken, metadata });
},
onExit: (exitError, metadata) => {
Log.info('[PlaidLink] Exit: ', false, { exitError, metadata });
ref.current.isVisible = false;
props.onExit();
},
});
return () => {
// eslint-disable-next-line react-hooks/exhaustive-deps
if (!ref.current.isVisible) return;
dismissLink();
}
}, []); ResultSimulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-09-17.at.19.34.41.movWhat alternative solutions did you explore? (Optional)None |
ProposalPlease re-state the problem that we are trying to solve in this issue.iOS: "Connect online with Plaid" popup is opened repeatedly after resuming the app What is the root cause of that problem?when notification open apps going to background mode. so when the notification closed app again active and reconnect api triggering
when reconnect api calling isLoadingReportData false and true
What changes do you think we should make in order to solve the problem?create new onyx key App/src/components/AddPlaidBankAccount.js Line 224 in c77814f
and
if ((!this.state.hasACHDataBeenLoaded || (isLoading && this.props.PLAID_CURRENT_EVENT != 'OPEN')) && shouldShowOfflineLoader && (shouldReopenOnfido || !this.requestorStepRef.current)) { and plaid popup loading mode there is no event so handle that
here we need add props.onEvent('OPEN', {}); and here no unmount handle we can dismiss the link on unmount
and finally, we should clear here
ResultUntitled.mp4 |
ProposalPlease re-state the problem that we are trying to solve in this issue.iOS - Connect online with Plaid popup is opened repeatedly What is the root cause of that problem?
App/src/components/PlaidLink/index.native.js Lines 13 to 28 in be0d9f4
and when we swipe down to view notifications, the app gets into background mode and when we swipe up the app activates again resulting in multiple pop overs
What changes do you think we should make in order to solve the problem?We need to apply following changes:
What alternative solutions did you explore? (Optional)NA Result:Simulator.Screen.Recording.-.iPhone.13.-.2023-09-18.at.06.18.06.mp4 |
ProposalPlease re-state the problem that we are trying to solve in this issue.PlaidLink popup is opened again after reopening the app from the background. What is the root cause of that problem?When the app reopens from the background,
App/src/pages/ReimbursementAccount/ReimbursementAccountPage.js Lines 363 to 371 in 9b302c9
This makes BankAccountStep mount again => Plaid popup open one more.
What changes do you think we should make in order to solve the problem?We use
To: const subStep = lodashGet(this.props.reimbursementAccount, 'achData.subStep', '');
if (... && subStep !== CONST.BANK_ACCOUNT.SETUP_TYPE.PLAID) ResultScreen.Recording.2023-09-18.at.9.21.19.PM.mov |
@kushu7, It seems like the pop-up window will appear and disappear multiple times? |
Hi, folks, so far, there are two questions that need clarifying:
|
@ntdiary can you check my proposal #27616 (comment) we can check the plaid event so that we can know modal is open or not and lodashGet(props.reimbursementAccount, 'achData.subStep', '') == CONST.BANK_ACCOUNT.SETUP_TYPE.PLAID |
@ntdiary I don't think my solution will cause it because |
📣 @situchan 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
📣 @tranvantoan-qn 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app! |
@adelekennedy There is a known issue with the external label double-assigning, so going to remove you 👍 |
@tgolen any update on the PR review? Thanks! |
merged and hit staging. GH will be updated upon next deploy |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.93-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2023-11-07. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
bump @situchan @CortneyOfstad to complete the above |
Payment Breakdown
|
No PR caused this regression. The bug existed always from the beginning. Regression Test Proposal (iOS, android)
|
@tgolen does that sound correct? |
Bump @tgolen on the question above ^^^ thanks! |
Yeah, that looks correct 👍 |
Thanks for the confirmation @tgolen! @pradeepmdk @situchan — I sent you both bonus proposals in Upwork — please let me know once you accept and I can get those paid ASAP. Thanks! |
@CortneyOfstad i accepted the offer |
Payments have been made successfully for the bonus amounts, so working on creating the regression test now 👍 |
Regression test created here — https://github.com/Expensify/Expensify/issues/336141. Closing as everything has been completed – thanks everyone! |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
"Connect online with Plaid" popup is opened only once
Actual Result:
Connect online with Plaid popup is opened repeatedly
NOTE : Can be reproduced with the steps 5. Put app to background and Resume app from background
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.70-5
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
popup-opened-repeatedly-02.MP4
popup-opened-repeatedly-01.MP4
WPNF8768.1.MP4
Expensify/Expensify Issue URL:
Issue reported by: @tranvantoan-qn
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694619647610939
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: