[HOLD for payment 2023-04-05] [$4000] Dev: Connect to bank account is not opening Sandbox page on dev server, redirects to real bank website

[kavimuru]

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:

1. Run the local web server.
2. Go to any workspace from the Settings page.
3. Click the last menu item Connect Bank account.
4. Click connect with Plaid .
5. Follow the flow.

Expected Result:

User should be navigated to sandbox environment to test bank accounts.

Actual Result:

Dev server is taking the user to real bank website which is impossible to test VBA flow on dev.

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web

Version Number: 1.2.21-4
Reproducible in staging?: Needs reproduction
Reproducible in production?: Needs reproduction
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: n/a
Expensify/Expensify Issue URL:
Issue reported by: @parasharrajat
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1667222549786869

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015d2f3c12b7b67693
• Upwork Job ID: 1623051744022671360
• Last Price Increase: 2023-02-23

[melvin-bot]

Triggered auto assignment to @adelekennedy (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

@adelekennedy Whoops! This issue is 2 days overdue. Let's get this updated quick!

[adelekennedy]

still reproducible - moving forward

[melvin-bot]

Triggered auto assignment to @stitesExpensify (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[adelekennedy]

I think this can be an external issue - @stitesExpensify for confirmation

[stitesExpensify]

I think this is going to have to be internal. AFAIK this is because of how our proxy works. The proxy points to production, when the sandbox page can only be reached from our staging servers.

CC @marcaaron @AndrewGable since you worked on the original proxy

[stitesExpensify]

I tried changing my .env file to point to staging.expensify.com but that just gave me CORS errors, so we may need to fix that too.

[adelekennedy]

ty @stitesExpensify will wait for confirmation on making this internal

[marcaaron]

Ah yeh hmm so I think this can actually be fixed externally, but requires jumping through some hoops..

Pre-requisites

• Proxy server is enabled via .env
• Staging version of the app is built
• Preferences > Test Preferences > Use Staging Server is checked

This doesn't work for secure because of this line here:

App/src/libs/HttpUtils.js

Lines 101 to 103 in ae3f424

	if (CONFIG.IS_IN_STAGING && shouldUseStagingServer) {
	apiRoot = shouldUseSecure ? CONFIG.EXPENSIFY.STAGING_SECURE_EXPENSIFY_URL : CONFIG.EXPENSIFY.STAGING_EXPENSIFY_URL;
	}

The proxy is at the root / see:

App/src/CONFIG.js

Line 21 in 952e5c3

const expensifyComWithProxy = getPlatform() === 'web' && useWebProxy ? '/' : expensifyURL;

So I think we need some way to:

• Use / to make the request to secure but maybe add something like a param &shouldUseSecure=true to the / url or a different path like /staging-api that the proxy can use to switch the hostname in the proxy request.
• Pipe the request to secure staging back to the client just like we do for regular production/staging server requests.

I am pretty sure that will work and we should not have to modify any headers.

[adelekennedy]

So should we take a gamble on making this external or keep it internal @stitesExpensify @marcaaron?

[stitesExpensify]

I'm thinking we can make it external and see if we get some proposals

[melvin-bot]

Triggered auto assignment to @CortneyOfstad (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

Triggered auto assignment to @greg-schroeder (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[greg-schroeder]

Okay @neil-marcellini - so should this be closed or remain open and attached to the new PR?

[greg-schroeder]

Bumping that question ☝️

[Prince-Mendiratta]

The PR we were waiting on has been deployed to prod today, I think we pick this issue back up now! Let's proceed once this question has been tackled.

cc @neil-marcellini @sobitneupane @mountiny

[neil-marcellini]

Sorry I missed your ping @greg-schroeder. Yes now that the PR is on production let's bring in the changes and move forward. We can keep this issue open.

[Prince-Mendiratta]

Latest PR is ready for review.

[luacmartins]

This still seems to be the case in the latest staging build. @Prince-Mendiratta can you please check it?

[Prince-Mendiratta]

@luacmartins I tried on latest main branch locally, working as expected. Tried on staging site and working well.

Can you please tell what you did so I can test it out too?

[luacmartins]

Our QA team didn't find any issues, maybe I missed something in the steps. We can disregard my comment above.

[MelvinBot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[MelvinBot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.91-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [Web Proxy] Use staging server for Web, Desktop when the "Use staging server" toggle is ON. #16432

If no regressions arise, payment will be issued on 2023-04-05. 🎊

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.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[Prince-Mendiratta]

Posting the timeline for this issue here for an overview and calculation of the bonus:

Issue Timeline Analysis
🐛 Issue created at: Mon, 31 Oct 2022 21:37:57 GMT
🧯 Help Wanted at: Tue, 07 Feb 2023 20:11:26 GMT
🕵🏻 Contributor assigned at: Mon, 27 Feb 2023 19:24:45 GMT
1️⃣ First PR created at: Mon, 27 Feb 2023 21:25:04 GMT
👾 Extra reviewers assigned at: Wed, 01 Mar 2023 00:17:44 GMT
⌚ PR merged at: Mon, 06 Mar 2023 11:11:27 GMT
⌛ Business Days Elapsed between assignment and PR merge: 4

Internal issue found, PR reverted, issue put on hold.
2️⃣ PR created at: Wed, 22 Mar 2023 23:33:05 GMT
🎯 PR merged at: Tue, 28 Mar 2023 11:09:40 GMT
⌛ Business Days Elapsed between issue put off hold and PR merge: 3

I'm not sure how the bonus should be calculated for this so I'll let the rest comment on this, posting this for easier analysis.

[greg-schroeder]

Thanks @Prince-Mendiratta, I was literally just looking at this to try and figure it out!

[parasharrajat]

I noticed one issue which might be a regression from it. Posting it on Slack with details and then I will update the link here. Please confirm if that is a regression from this PR.

[parasharrajat]

If there was an issue and PR was reverted then it does not qualify for the bonus. Technically, the issue should be pointed out and fixed before the merge, not after the merge. After the merge issues are considered regressions.

[greg-schroeder]

Yeah, even though the PR was merged within 3 business days after the issue was taken off hold, I think we should factor in the entire timeline of events. I'm inclined to just pay the baseline (as long we don't in fact have a regression).

[Prince-Mendiratta]

Just want to point out that the issue caused by merging of the PR broke the internal dev setup, which neither I or @sobitneupane had access to.

We decided to put the issue on hold as another PR which was in the work. More details on this in the slack thread.

[parasharrajat]

But there are internal reviewers as well on the PR. Aside from it, the first PR was not merged in 3 days so still it does not qualify.

[greg-schroeder]

Sent offers to @parasharrajat, @sobitneupane, @Prince-Mendiratta!