[Staging API] To Use Staging Server or not to use staging server

[Prince-Mendiratta]

Details

With this PR, we are adding the ability to map all requests to production / staging API depending on the "Use staging server" toggle in user preferences. This change only affects the Development and Staging Environment.

Fixed Issues

$ #12315
PROPOSAL: #12315 (comment)

Tests

.env is not defined

1. Login to ND.
2. Go to Settings > Preferences > toggle the Use staging server toggle ON.
3. Go to Settings > Payments > Add payment Method > Bank Account.
4. Ensure that the Sandbox mode of Plaid is initiated. On Web and Desktop, this can be seen very clearly as the disclaimer is shown at the bottom. For native devices and mWeb, a good way to determine if we are in sandbox or not is the presence of the reCaptcha after selecting the bank institution. If it is present, then we are in production mode. If not, we are in sandbox mode. Another method to determine this on native and mWeb is that the flow redirects to the sandbox URL so it can be seen pretty clearly there as well.
5. Continue to add account normally in sandbox mode with any details. Notice that the account is added successfully after completing the flow.
6. Go to Settings > Preferences > toggle the Use staging server toggle OFF.
7. Go to Settings > Payments > Add payment Method > Bank Account.
8. Ensure that the Production mode of Plaid is initiated. Ways to confirm this are same as mentioned above.
9. Exit out of the production plaid flow
10. Open any chat.
11. Add a new attachment. Upload the attachment. Open it.
12. Ensure all action in step 11 happen seamlessly.

Expected Console Output

.env is defined
This is basically to confirm that all the requests will be proxied to the server depending on the contents of the .env file. To test this, create a .env file with this content:

EXPENSIFY_URL=https://staging.expensify.com/
STAGING_EXPENSIFY_URL=https://staging.expensify.com/
STAGING_SECURE_EXPENSIFY_URL=https://staging-secure.expensify.com/

1. Login to ND.
2. Go to Settings > Preferences > toggle the Use staging server toggle ON.
3. Go to Settings > Payments > Add payment Method > Bank Account.
4. Ensure that the Sandbox mode of Plaid is initiated. Ways to confirm this are same as mentioned above.
5. Continue to add account normally in sandbox mode with any details. Notice that the account is added successfully after completing the flow.
6. Go to Settings > Preferences > toggle the Use staging server toggle OFF.
7. Go to Settings > Payments > Add payment Method > Bank Account.
8. Ensure that the Sandbox mode of Plaid is initiated. Ways to confirm this are same as mentioned above.
9. Continue to add account normally in sandbox mode with any details. Notice that the account is added successfully after completing the flow.

Expected Console Output

Note: There is a known issue in Android where when repeating the above tests will lead to an error. This has been reported in slack and will be tackled in a separate issue. The above tests should work well for all other platforms.

Note 2: You might see an "Internal React error: Attempted to capture" error while testing but that is being tackled in #13917 and is not related to current changes.

• Verify that no errors appear in the JS console

Offline tests

N/A, doesn't affect the offline functionality, should behave the same as expected behaviour when offline.

QA Steps

This doesn't affect the production behaviour. For staging,

1. Login to ND.
2. Go to Settings > Preferences > toggle the Use staging server toggle ON.
3. Go to Settings > Payments > Add payment Method > Bank Account.
4. Ensure that the Sandbox mode of Plaid is initiated. On Web and Desktop, this can be seen very clearly as the disclaimer is shown at the bottom. For native devices and mWeb, a good way to determine if we are in sandbox or not is the presence of the reCaptcha after selecting the bank institution. If it is present, then we are in production mode. If not, we are in sandbox mode. Another method to determine this on native and mWeb is that the flow redirects to the sandbox URL so it can be seen pretty clearly there as well.
5. Continue to add account normally in sandbox mode with any details. Notice that the account is added successfully after completing the flow.
6. Go to Settings > Preferences > toggle the Use staging server toggle OFF.
7. Go to Settings > Payments > Add payment Method > Bank Account.
8. Ensure that the Production mode of Plaid is initiated. Ways to confirm this are same as mentioned above.
9. Continue to add account normally in sandbox mode with any details. Notice that the account is added successfully after completing the flow.
10. Open any chat.
11. Add a new attachment. Upload the attachment. Open it.
12. Ensure all action in step 11 happen seamlessly.

• Verify that no errors appear in the JS console

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I followed proper code patterns (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
  • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • I verified the JSDocs style guidelines (in STYLE.md) were followed
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• I verified that if a function's arguments changed that all usages have also been updated correctly
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • The file is named correctly
  • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
  • The only data being stored in the state is data necessary for rendering and nothing else
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • All JSX used for rendering exists in the render method
  • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

without .env defined

Web

2023-02-28.02-13-17.mp4

Mobile Web - Chrome

2023-02-28.02-37-30.mp4

Mobile Web - Safari

mWeb-Safari.mp4

Desktop

desktop.mp4

iOS

2023-02-28.02-06-56.mp4

Android

2023-02-28.02-35-08.mp4

.env defined as mentioned in above tests

Web

2023-02-28.18-05-34.mp4

Mobile Web - Chrome

2023-02-28.18-13-08.mp4

Mobile Web - Safari

2023-02-28.20-48-37.mp4

Desktop

2023-02-28.20-42-09.mp4

iOS

2023-02-28-20-32-23_3RJY0UXT.mp4

Android

2023-02-28.18-14-36.mp4

[MelvinBot]

@neil-marcellini @sobitneupane One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

[sobitneupane]

Screenshots/Videos

Web

Screen.Recording.2023-02-28.at.17.01.51.mov

Mobile Web - Chrome

Screen.Recording.2023-02-28.at.17.17.44.mov

Mobile Web - Safari

Screen.Recording.2023-02-28.at.17.13.32.mov

Desktop

Screen.Recording.2023-02-28.at.17.08.43.mov

iOS

Screen.Recording.2023-02-28.at.17.24.10.mov

Android

Screen.Recording.2023-02-28.at.17.29.56.mov

[sobitneupane]

@Prince-Mendiratta Can you please add screenshots for the condition ".env is defined" in Tests?

[Prince-Mendiratta]

@sobitneupane Sure, will do so. Is it required for all platforms?

[sobitneupane]

@Prince-Mendiratta I think so. Web and Native behave differently. So, it is good to have screenshots for all platforms.

[Prince-Mendiratta]

Got it, I'll ping you once I've added the recordings.

[Prince-Mendiratta]

@sobitneupane updated with tests!

[neil-marcellini]

Thanks! The code tests well and it's looking pretty good. I would like to see a few tweaks to make the code a bit easier to understand.

Also, I don't think we want to ask people to add a bank account with Plaid on production for testing. I think it should be enough to see that Plaid links to the production environment. Adding a bank account in the sandbox is a good idea however. I'm curious, what credentials do you use to test in the Sandbox mode?

[neil-marcellini]

cc @iwiznia @mountiny in case you guys want to take a look since you participated in the Slack conversation.

[sobitneupane]

.env.staging file has the following content:

NEW_EXPENSIFY_URL=https://staging.new.expensify.com/
SECURE_EXPENSIFY_URL=https://secure.expensify.com/
EXPENSIFY_URL=https://www.expensify.com/
EXPENSIFY_PARTNER_NAME=chat-expensify-com
EXPENSIFY_PARTNER_PASSWORD=e21965746fd75f82bb66
PUSHER_APP_KEY=268df511a204fbb60884
USE_WEB_PROXY=false
ENVIRONMENT=staging
SEND_CRASH_REPORTS=true

We are expecting to have EXPENSIFY_URL, STAGING_EXPENSIFY_URL , STAGING_SECURE_EXPENSIFY_URL to be set on .env.staging. I am little confused why we don't have STAGING_EXPENSIFY_URL and STAGING_SECURE_EXPENSIFY_URL on .env.staging as of now.

EXPENSIFY_URL=https://staging.expensify.com/
STAGING_EXPENSIFY_URL=https://staging.expensify.com/
STAGING_SECURE_EXPENSIFY_URL=https://staging-secure.expensify.com/

cc: @neil-marcellini

[Prince-Mendiratta]

@sobitneupane As far as I understand, the NEW_EXPENSIFY_URL is the website's URL and the EXPENSIFY_URL is the URL used to hit the API. On staging, all requests are sent to the prod server by default.

As for the staging and secure staging URL, even if they are not defined in the staging .env, they are automatically set with lodash with this configuration -

App/src/CONFIG.js

Lines 16 to 17 in 44c19cb

	const stagingExpensifyURL = Url.addTrailingForwardSlash(lodashGet(Config, 'STAGING_EXPENSIFY_URL', 'https://staging.expensify.com/'));
	const stagingSecureExpensifyUrl = Url.addTrailingForwardSlash(lodashGet(Config, 'STAGING_SECURE_EXPENSIFY_URL', 'https://staging-secure.expensify.com/'));

We only need to set the .env file if the contributor specifically needs to set different production and staging URLs, as would be done by internal employees in this manner:

EXPENSIFY_URL=https://www.expensify.com.dev/
STAGING_EXPENSIFY_URL=https://staging.expensify.com.dev/
STAGING_SECURE_EXPENSIFY_URL=https://staging-secure.expensify.com.dev/

[sobitneupane]

@Prince-Mendiratta Thanks. I was just looking for confirmation from CME.

[sobitneupane]

@neil-marcellini

I'm curious, what credentials do you use to test in the Sandbox mode?

username: user_good
password: pass_good

[mountiny]

Oof, I think we now how 3 PRs touching a similar code #14944 and #15178

cc @kidroca @MonilBhavsar would you want to review this one too?

[kidroca]

Added some notes

[neil-marcellini]

Looks really close to being ready. I agree with @kidroca's comment about moving the api map constant.

[mountiny]

Agreed with kidroca comment, had one other question

[Prince-Mendiratta]

@neil-marcellini I tried to generate a production build for web and used serve dist to run it locally but I kept getting the CORS error, any guide on how to test production build locally?

Also, is it required to test the production build for all platforms?

[mountiny]

Looks good to me, although always anxious about these changes, gotta make sure to test everything really well after deploy (shouldnt be an issue, but we had in past let regressions slip because we did not test properly and it was not easy to spot)

[Prince-Mendiratta]

I tested the production builds for android, iOS, desktop and it works well. Since the staging toggle is not visible, it doesn't affect the functionality and does not break anything.

I tried out the staging build too for the above platforms and it was working well as expected including the tests mentioned above.

cc @neil-marcellini @mountiny

[kidroca]

I have some notes on the regex usage

I lean towards NAB, but perhaps it would be simpler if we don't use regex

[Prince-Mendiratta]

gentle bump @neil-marcellini

[mountiny]

Thanks @Prince-Mendiratta @kidroca I will go ahead and merge this since @neil-marcellini already gave his approval before.

[OSBotify]

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

[mountiny]

gonna have to revert this as its breaking internal dev setup

[Prince-Mendiratta]

@mountiny can you please tell the issue? Is it because of the requests not hitting the .dev domain?

[mountiny]

@Prince-Mendiratta correct, for internal engineers, the requests should be hitting local dev VM, this is my env:

NEW_EXPENSIFY_URL=https://new.expensify.com/
SECURE_EXPENSIFY_URL=https://secure.expensify.com.dev/
EXPENSIFY_URL=https://www.expensify.com.dev/
EXPENSIFY_PARTNER_NAME=chat-expensify-com
EXPENSIFY_PARTNER_PASSWORD=removed
PUSHER_APP_KEY=removed
SECURE_NGROK_URL=removed
NGROK_URL=removed
USE_NGROK=true
USE_WEB_PROXY=false
PUSHER_DEV_SUFFIX=removed

[Prince-Mendiratta]

@mountiny You'll have to modify the .env slightly to include this case too:

STAGING_EXPENSIFY_URL=https://staging.expensify.com.dev/
STAGING_SECURE_EXPENSIFY_URL=https://staging-secure.expensify.com.dev/

Can you please try with these and see if the issue is resolved?

Thanks.

[Prince-Mendiratta]

My analysis based on the limited information from the above screenshot, looks like the local storage has the shouldUseStagingServer toggled on and the login request is hitting the staging API. Since the .env is not configured to match .dev domains, it's hitting the actual staging API and thus, the error.

As per a summary of the slack discussion, we should send in a disclaimer for internal employees to update their .env files to reflect the changes with this PR.
We should also potentially update the example .env files in the repo.

[mountiny]

no user key in the onyx so that wont be the issue

@Prince-Mendiratta Can we achieve this without everyone needing to change their local env?

[Prince-Mendiratta]

no user key in the onyx so that wont be the issue

I see, I wonder why the app is hitting the staging API then. 🤔

@Prince-Mendiratta Can we achieve this without everyone needing to change their local env?

@mountiny as we discussed in the slack thread that the easiest way to implement this would be to let the .env determine which endpoint to hit, do we want to revisit that decision?

[mountiny]

@Prince-Mendiratta it does not work even with the updated env.

In a call now, can you please create a thread in Slack to discuss this/test this easier :) thanks 🙇

[kidroca]

This seems to be the reason you're hitting staging @mountiny

The default should always be false now be false unless we're on staging

[OSBotify]

🚀 Deployed to staging by https://github.com/mountiny in version: 1.2.79-0 🚀

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

[mvtglobally]

Has this PR been reverted? Can we check it off? #15678

[neil-marcellini]

@mvtglobally yes it was reverted here #15678

[OSBotify]

🚀 Deployed to production by https://github.com/roryabraham in version: 1.2.79-0 🚀

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