Use app environment to determine olddot URL and update staging env #14944

MonilBhavsar · 2023-02-08T13:45:43Z

cc @AndrewGable

Details

Update staging env file to point to staging olddot
Use Environment to determine olddot domain/URL

Fixed Issues

$ #14820
PROPOSAL: https://expensify.slack.com/archives/C03TQ48KC/p1675664867491769

Tests

Use case 1

Go to settings > Workspace.
Go to Reimburse Expense > Click on "View all receipts" and confirm you're redirected to appropriate olddot (if using staging then staging, if using production, then production)
Go to Pay Bills > Click on "View all bills" and confirm you're redirected to appropriate olddot (if using staging then staging, if using production, then production)
Go to Send Invoices > Click on "Send Invoices" and "View all invoices" and confirm you're redirected to appropriate olddot (if using staging then staging, if using production, then production)

Use case 2

Have 2FA disabled on olddot.

Go to Workspace > Connect bank account and start the process of setting up bank account in VERIFYING state
Go till Verifying state and confirm the 2FA prompt
Click on "Secure Account" and confirm you're redirected to appropriate olddot (if using staging config then staging, if using production config, then production)

Now, updated .env file and added ENVIRONMENT=staging
Tested both use cases above, ensured it redirects to staging olddot URL

Similarly, updated .env file and added ENVIRONMENT=production
Tested both use cases above, ensured it redirects to production olddot URL

For mobile native, updated getEnvironment() in src/libs/Environment/getEnvironment/index.native.js to return early and simulate staging env

function getEnvironment() {
    return new Promise((resolve) => {
        // If we've already set the environment, use the current value
+        return resolve(CONST.ENVIRONMENT.STAGING);

        if (environment) {

Use case 3 - building Staging app

Followed this SO to point to staging app. Updated env file similar to .env.staging file to point to staging API.

Copy the contents of the .env.staging file to the .env file.

Run npm run desktop

In preferences > disabled "Use staging server", so that API points to production
Go to Reimburse Expense > Click on "View all receipts" and confirm you're redirected to appropriate olddot (if using staging then staging, if using production, then production)

Note:

(For internal engineer) For testing on mobile web, update DEV_EXPENSIFY_URL in CONST.js to your ngrok URL
Verify that no errors appear in the JS console

Offline tests

QA Steps

Use case 1

Go to settings > Workspace.
Go to Reimburse Expense > Click on "View all receipts" and confirm you're redirected to appropriate olddot (if using staging then staging, if using production, then production)
Go to Pay Bills > Click on "View all bills" and confirm you're redirected to appropriate olddot (if using staging then staging, if using production, then production)
Go to Send Invoices > Click on "Send Invoices" and "View all invoices" and confirm you're redirected to appropriate olddot (if using staging then staging, if using production, then production)

Use case 2

Have 2FA disabled on olddot.

Go to Workspace > Connect bank account and start the process of setting up bank account in VERIFYING state
Go till Verifying state and confirm the 2FA prompt
Click on "Secure Account" and confirm you're redirected to appropriate olddot (if using staging then staging, if using production, then production)

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

Screen.Recording.2023-02-15.at.6.02.06.PM.mov

Mobile Web - Chrome

Screen.Recording.2023-02-15.at.7.22.28.PM.mov

Mobile Web - Safari

Screen.Recording.2023-02-15.at.7.06.05.PM.mov

Desktop

Screen.Recording.2023-02-15.at.6.24.41.PM.mov

iOS

Screen.Recording.2023-02-15.at.7.11.15.PM.mov

Android

Screen.Recording.2023-02-16.at.12.54.36.AM.mov

MelvinBot · 2023-02-15T12:36:27Z

@rushatgabhane @Gonals 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]

rushatgabhane · 2023-02-17T07:44:01Z

asked @s77rt to review this

MonilBhavsar · 2023-02-17T09:36:29Z

Should I assign @s77rt to the issue as C+ as well as for the review?

s77rt · 2023-02-17T09:50:33Z

@rushatgabhane Sorry, but I have not been asked for that 😅 Perhaps you mis-tagged?

mountiny · 2023-02-17T10:45:30Z

Hey! Noting we have a refactor PR here which is also looking to this part of the code #15178

@kidroca would you want to review this as well to make sure it is aligned with the changes you will be making?

kidroca · 2023-02-17T11:07:18Z

@mountiny

@kidroca would you want to review this as well to make sure it is aligned with the changes you will be making?

Yeah, it seems we're doing a similar thing and IMO we should pack related functionality to ApiUtils (or UrlUtils) instead, so that URLs are retrieved/managed from one place instead of be scatter through Environment and other places

I want to make a few notes on the code as well (will post a review in a bit)

kidroca

Added a few notes

Additionally I think we should rename ApiUtils from my PR to UrlUtils and handle API urls and Old Dot URLs there

kidroca · 2023-02-17T11:15:37Z

.env.staging

 NEW_EXPENSIFY_URL=https://staging.new.expensify.com/
 SECURE_EXPENSIFY_URL=https://secure.expensify.com/
-EXPENSIFY_URL=https://www.expensify.com/
+EXPENSIFY_URL=https://staging.expensify.com/


I think we can simplify .env.{name} configs by moving the URLs out of them

Native uses .env.production to build the production as well as the staging app

We have a API toggle that tries to toggle between the URL set here (EXPENSIFY_URL) and the staging URL. So if we set EXPENSIFY_URL to https://staging.expensify.com/ this would stop working - current logic expects EXPENSIFY_URL to be the prod url (e.g. trace how URL_API_ROOT and how it's used in src/libs/HttpUtils.js

I propose to declare all the URLs in one place, like you already do in src/CONST.js
Then use environment information to select the correct underlying API, like I try to do in: #15178

This way we also reduce copy pasted code across .env files

I think we can simplify .env.{name} configs by moving the URLs out of them
I propose to declare all the URLs in one place, like you already do in src/CONST.js
This way we also reduce copy pasted code across .env files

Yes, I like this idea. Let's do it. Just for visibility we can post this in slack.

So if we set EXPENSIFY_URL to https://staging.expensify.com/ this would stop working - current logic expects EXPENSIFY_URL to be the prod url (e.g. trace how URL_API_ROOT and how it's used in src/libs/HttpUtils.js

Sorry, I didn't get this. Could you please explain

On Staging, there's a Settings -> Preferences -> Test Preferences menu, where you can toggle between the STAGING and PROD APIs, and this depends on EXPENSIFY_URL (from .env.staging) to point to the PROD api

See how the code works here

App/src/libs/HttpUtils.js

Lines 110 to 114 in 6971b98

let apiRoot = shouldUseSecure ? CONFIG.EXPENSIFY.SECURE_EXPENSIFY_URL : CONFIG.EXPENSIFY.URL_API_ROOT;

if (shouldUseStagingServer(stagingServerToggleState)) {

apiRoot = shouldUseSecure ? CONFIG.EXPENSIFY.STAGING_SECURE_EXPENSIFY_URL : CONFIG.EXPENSIFY.STAGING_EXPENSIFY_URL;

}

URL_API_ROOT is resolved from EXPENSIFY_URL

With the change you propose the code would evaluate to something like this:

HttpUtil on Web Staging

let apiRoot = 'https://staging.expensify.com/'; if (shouldUseStagingServer(stagingServerToggleState)) { apiRoot = 'https://staging.expensify.com/'; }

Right now the code works correctly because EXPENSIFY_URL is https://www.expensify.com/
so we have this

HttpUtil on Web Staging

let apiRoot = 'https://www.expensify.com/'; if (shouldUseStagingServer(stagingServerToggleState)) { apiRoot = 'https://staging.expensify.com/'; }

Ah got it! Thanks for explanation.

Thinking more about this. Looks like the two features -

a). Redirecting to appropriate olddot app and
b). Making API calls from app to appropriate domain should behave in different ways.

For a). the redirection from newdot to olddot should occur to corresponding environment. i.e. staging to staging, prod to prod and dev to dev.
Whereas in b). the API calls should be made to staging server if "user staging server" setting is toggled on, else should be made to production server.

So, I have reverted the change in .env.staging file that was breaking this behavior and is unrelated to this PR. Thanks for catching it. I am +1 to move all this URL logic to a lib and remove them from env files, but I think it should be done in a different PR. We can discuss that and make a new PR to cleanup the logic.

kidroca · 2023-02-17T11:16:52Z

src/CONST.js

@@ -279,6 +279,9 @@ const CONST = {
    CFPB_PREPAID_URL: 'https://cfpb.gov/prepaid',
    STAGING_NEW_EXPENSIFY_URL: 'https://staging.new.expensify.com',
    NEWHELP_URL: 'https://help.expensify.com',
+    DEV_EXPENSIFY_URL: 'https://www.expensify.com.dev',


IIRC DEV_EXPENSIFY_URL is the same as PROD

This address doesn't work: https://www.expensify.com.dev

You need to have a local VM for this to work so this works only for internal developers

Yes, dev is different from prod. It links to internal engineer's VM.

To me the name DEV_EXPENSIFY_URL is confusing, because in dev we use the PROD url
If this is meant only as a reference to OLD DOT, perhaps it should be listed as DEV_OLD_DOT_URL

Since this won't work for external contributors, perhaps there should be a comment about it

Maybe rename this to INTERNAL_DEV_EXPENSIFY_URL

Let's update it to Internal as contributors dev use Prod URL. 👍

kidroca · 2023-02-17T11:26:45Z

src/libs/actions/Link.js

-        return `${CONFIG.EXPENSIFY.EXPENSIFY_URL}${url}${hasHashParams || hasURLParams ? '&' : '?'}${params}`;
+        return Environment.getOldDotEnvironmentURL()
+            .then((environmentURL) => {
+                const oldDotDomain = Url.addTrailingForwardSlash(environmentURL || CONFIG.EXPENSIFY.EXPENSIFY_URL);


Is it possible for environmentURL to be null | undefined here? Why?

Is it OK to fallback to the production address? If yes, why not make getOldDotEnvironmentURL() have the fallback

Is it possible for environmentURL to be null | undefined here? Why?

Usually not. We can remove it. I added just for a safety. I see promise is always resolved.

If for some reason getOldDotEnvironmentURL doesn't work as expected, the fallback here would hide the bug instead of point it out

Agree, I can't think of a case where it would fail. I am going to remove it.

src/libs/actions/Link.js

kidroca · 2023-02-17T14:16:03Z

src/libs/Environment/Environment.js

+/**
+ * Get the corresponding oldDot URL based on the environment we are in
+ *
+ * @returns {Promise}
+ */
+function getOldDotEnvironmentURL() {


Do we know if this should depend on the staging api toggle (see Preferences / Settings / Test Settings)

Should this return the PROD Old Dot URL, if the API toggle is set to production?

Yes!

if the API toggle is set to production?

I guess this is when "Use staging server" is toggled off

Wait, looking into the above problem, we should clarify this.

if the API toggle is set to production?

I guess this is when "Use staging server" is toggled off

Correct

Do we know if this should depend on the staging api toggle (see Preferences / Settings / Test Settings)

Wait, looking into the above problem, we should clarify this.

I'm not sure either, can you ask on the issue related to the current PR, or open a slack thread about it

kidroca · 2023-02-17T14:16:35Z

src/libs/Environment/Environment.js

+    return new Promise((resolve) => {
+        getEnvironment()
+            .then(environment => resolve(OLDDOT_ENVIRONMENT_URLS[environment]));
+    });


Wrapping with new Promise((resolve => ) is unnecessary - getEnvironment already returns a promise you can just do this:

return getEnvironment() .then(environment => OLDDOT_ENVIRONMENT_URLS[environment]);

kidroca · 2023-02-17T14:18:25Z

src/libs/Environment/Environment.js

+/**
+ * Get the corresponding oldDot URL based on the environment we are in
+ *
+ * @returns {Promise}


Suggested change

* @returns {Promise}

* @returns {Promise<String>}

Returns a Promise that resolves to a string

mountiny · 2023-02-22T18:38:35Z

@MonilBhavsar Bumping this one, are we waiting on some discussion to conclude?

MonilBhavsar · 2023-02-27T10:24:11Z

Sorry, for some delay, I have addressed reviews. I think we should discuss to cleanup the URL logic and get rid of different and confusing URL_ variables and URL's in env file, but I think it should be a different issue and PR #14944 (comment)

MonilBhavsar · 2023-03-07T09:45:38Z

@mountiny @kidroca @Gonals looks like we're good to ahead here?

src/libs/actions/Link.js

mountiny

Looks good to me now but gotta ask @rushatgabhane or some C+ for doing the checklist

mountiny · 2023-03-08T15:18:17Z

Adding @s77rt to go though the checklist and testing steps

mountiny · 2023-03-08T15:31:22Z

The bridge works fine on dev for me

s77rt · 2023-03-08T16:38:52Z

src/libs/Environment/Environment.js

+ * @returns {Promise<string>}
+ */
+function getOldDotEnvironmentURL() {
+    return getEnvironment()


Using getEnvironment will break olddot urls for those who don't use .env file. The olddot url will fallback to the dev url which is not accessible.

But how one can not have .env file. I think you meant contributors.

Yes, external contributors are setup so that they don't need an .env file, even if they have one, they won't be able to access https://www.expensify.com.dev (this value comes directly from CONST and not from .env)

Is it OK if this functionality doesn't work for external contributors, when they are using the DEV build?

Since external contributors are using the PROD API (In DEV via proxy), perhaps this link should point to https://www.expensify.com for external contributors during DEV

This is a low priority bug, it's only at DEV time, if we can fix this with the least work it would be great otherwise we can just point this on the README file so contributors can overwrite the getOldDotEnvironmentURL for testing.

Got it,
So technically this doesn't sound like a bug.

As @s77rt, We can ask contributors to make it work by overriding the URL in CONST or updating getOldDotEnvironmentURL()

Not sure, if we should add it to README

I think we can post in slack and give a heads up to contributors

Yeah README is an overkill for such minor detail that contributors may not even have to deal with. Let's do nothing for now, if we get reports on this we may consider adding it to README then.

Sounds good! Thank you!

Posting this on Slack seems as a good idea, let's do it.

s77rt · 2023-03-08T17:28:07Z

I can't really modify the .env due to an ongoing CORS issue (web/proxy). Testing by overwriting getOldDotEnvironmentURL

-        .then(environment => OLDDOT_ENVIRONMENT_URLS[environment]);
+        .then(environment => OLDDOT_ENVIRONMENT_URLS["production"]);

s77rt · 2023-03-08T20:15:33Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible 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 checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (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
- 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 verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
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
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.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Case1:
- Staging:
  
  web.mp4
- Production:
  
  web.mp4

Mobile Web - Chrome

Case1:
- Staging:
  
  mweb-chrome.mp4
- Production:
  
  mweb-chrome.mp4

Mobile Web - Safari

Case1:
- Staging:
  
  mweb-safari.mp4
- Production:
  
  mweb-safari.mp4

Desktop

Case1:
- Staging:
  
  desktop.mp4
- Production:
  
  desktop.mp4

iOS

Case1:
- Staging:
  
  ios.mp4
- Production:
  
  ios.mp4

Android

Case1:
- Staging:
  
  android.mp4
- Production:
  
  android.mp4

s77rt · 2023-03-08T20:20:37Z

Overall looking good just need to address the raised concern here #14944 (comment).

I was only able to test case 1. Case 2 I didn't get the 2FA prompt although it's disabled and case 3 seems internal, right?

MonilBhavsar · 2023-03-09T04:37:29Z

@s77rt I replied to that comment.
Hmm strange, you should have received the prompt. Do you have a video?

case 3 seems internal, right?

It can be tested externally. I updated the steps to build the staging app and point to staging API.

s77rt · 2023-03-09T11:12:42Z

For case 2:
Looks the VBBA didn't go through the VERIFYING state? Using Alberta's details here.

Kooha-2023-03-09-11-54-24.mp4

For case 3:
This one is failing, maybe it's expected it to fail since the "Use staging server" switch is for outgoing requests and have nothing to do with building the olddot urls. If this is required then we should update getOldDotEnvironmentURL to take into account shouldUseStagingServer.

Kooha-2023-03-09-12-10-41.mp4

MonilBhavsar · 2023-03-09T11:48:17Z

For case 2:
Looks the VBBA didn't go through the VERIFYING state? Using Alberta's details #11512 (comment).

Ah, yes case 2 needs to be internal to get that account in VERIFYING state and see 2FA prompt

MonilBhavsar · 2023-03-09T11:50:17Z

For case 3:
This one is failing, maybe it's expected it to fail since the "Use staging server" switch is for outgoing requests and have nothing to do with building the olddot urls. If this is required then we should update getOldDotEnvironmentURL to take into account shouldUseStagingServer.

I think that is expected behavior. We discussed and decided that "Use staging server" switch should have no effect on redirection.

s77rt · 2023-03-09T11:54:26Z

@MonilBhavsar Great, thanks for the confirmation. Can you let me know what are the next steps now?

MonilBhavsar · 2023-03-09T12:20:29Z

Thanks! Let's merge it then 🚀

s77rt · 2023-03-09T12:23:04Z

@MonilBhavsar We didn't get a final decision on this one #14944 (comment)

OSBotify · 2023-03-09T12:25:41Z

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

MonilBhavsar · 2023-03-09T12:37:13Z

Ah sorry, I missed that. Commented in the thread, but looks like no action will be required

OSBotify · 2023-03-10T14:50:12Z

🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 1.2.82-0 🚀

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

OSBotify · 2023-03-13T12:38:28Z

🚀 Deployed to production by https://github.com/Julesssss in version: 1.2.82-4 🚀

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

s77rt

@MonilBhavsar

s77rt · 2023-03-13T23:00:51Z

src/libs/actions/Link.js

+
+                // If the URL contains # or ?, we can assume they don't need to have the `?` token to start listing url parameters.
+                return `${oldDotDomain}${url}${hasHashParams || hasURLParams ? '&' : '?'}${params}`;
+            });
    }

    if (isNetworkOffline) {


@MonilBhavsar A bit late, but don't we need to make changes here? Linking.openURL expects a string not a promise.

buildOldDotURL().then((oldDotUrl) => { Linking.openURL(oldDotUrl); });

We missed it. It was fixed here #15923

Oh too late then. Sorry about that

Update staging URL

6ae89fc

MonilBhavsar self-assigned this Feb 8, 2023

MonilBhavsar added 2 commits February 15, 2023 17:43

Use environment to determine the URL

2e9b031

Merge branch 'main' into monil-updateStagingENV

d6767d1

MonilBhavsar marked this pull request as ready for review February 15, 2023 12:35

MonilBhavsar requested a review from a team as a code owner February 15, 2023 12:35

melvin-bot bot requested review from Gonals and rushatgabhane and removed request for a team February 15, 2023 12:36

MonilBhavsar changed the title ~~Use olddot staging URL in staging environment file~~ Use app environment to determine olddot URL and update staging env Feb 15, 2023

Gonals previously approved these changes Feb 16, 2023

View reviewed changes

mallenexpensify mentioned this pull request Feb 16, 2023

[HOLD for payment 2023-03-20] [$1000] Web - VBA - Clicking secure account opens production OD and after set up redirects to ND login #14820

Closed

6 tasks

kidroca suggested changes Feb 17, 2023

View reviewed changes

kidroca reviewed Feb 17, 2023

View reviewed changes

kidroca mentioned this pull request Feb 21, 2023

Refactor: Create libs/ApiUtils #15178

Merged

74 tasks

Merge branch 'main' into monil-updateStagingENV

b6dd94e

kidroca mentioned this pull request Feb 27, 2023

[HOLD for payment 2023-03-28] Make sure attachments are fetched from appropriate servers #15087

Closed

Address reviews

bb0e55b

MonilBhavsar dismissed Gonals’s stale review via bb0e55b February 27, 2023 09:56

mountiny requested review from kidroca and Gonals and removed request for kidroca February 27, 2023 11:47

mountiny mentioned this pull request Mar 1, 2023

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

Merged

54 tasks

kidroca previously approved these changes Mar 7, 2023

View reviewed changes

mountiny requested changes Mar 7, 2023

View reviewed changes

src/libs/actions/Link.js Outdated Show resolved Hide resolved

Lint fix

5a980d2

MonilBhavsar dismissed kidroca’s stale review via 5a980d2 March 7, 2023 18:53

mountiny approved these changes Mar 8, 2023

View reviewed changes

mountiny requested a review from s77rt March 8, 2023 15:18

s77rt reviewed Mar 8, 2023

View reviewed changes

MonilBhavsar merged commit 8ccfa41 into main Mar 9, 2023

MonilBhavsar deleted the monil-updateStagingENV branch March 9, 2023 12:21

OSBotify mentioned this pull request Mar 10, 2023

Deploy Checklist: New Expensify 2023-03-10 #15836

Closed

16 tasks

s77rt reviewed Mar 13, 2023

View reviewed changes

s77rt mentioned this pull request Mar 18, 2023

Handle error in workspace's bank account page #15394

Merged

55 tasks

	let apiRoot = shouldUseSecure ? CONFIG.EXPENSIFY.SECURE_EXPENSIFY_URL : CONFIG.EXPENSIFY.URL_API_ROOT;

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

Use app environment to determine olddot URL and update staging env #14944

Use app environment to determine olddot URL and update staging env #14944

Conversation

MonilBhavsar commented Feb 8, 2023 • edited Loading

Details

Fixed Issues

Tests

Use case 1

Use case 2

Use case 3 - building Staging app

Offline tests

QA Steps

Use case 1

Use case 2

PR Author Checklist

Screenshots/Videos

MelvinBot commented Feb 15, 2023

rushatgabhane commented Feb 17, 2023

MonilBhavsar commented Feb 17, 2023

s77rt commented Feb 17, 2023

mountiny commented Feb 17, 2023

kidroca commented Feb 17, 2023 • edited Loading

kidroca left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HttpUtil on Web Staging

HttpUtil on Web Staging

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MonilBhavsar Feb 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mountiny commented Feb 22, 2023

MonilBhavsar commented Feb 27, 2023 • edited Loading

MonilBhavsar commented Mar 7, 2023

mountiny left a comment

Choose a reason for hiding this comment

mountiny commented Mar 8, 2023

mountiny commented Mar 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kidroca Mar 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

s77rt commented Mar 8, 2023 • edited Loading

s77rt commented Mar 8, 2023

Reviewer Checklist

Screenshots/Videos

s77rt commented Mar 8, 2023

MonilBhavsar commented Mar 9, 2023

s77rt commented Mar 9, 2023

MonilBhavsar commented Mar 9, 2023

MonilBhavsar commented Mar 9, 2023

s77rt commented Mar 9, 2023

MonilBhavsar commented Mar 9, 2023

s77rt commented Mar 9, 2023

OSBotify commented Mar 9, 2023

MonilBhavsar commented Mar 9, 2023

OSBotify commented Mar 10, 2023

MonilBhavsar commented Feb 8, 2023 •

edited

Loading

kidroca commented Feb 17, 2023 •

edited

Loading

MonilBhavsar Feb 20, 2023 •

edited

Loading

MonilBhavsar commented Feb 27, 2023 •

edited

Loading

kidroca Mar 9, 2023 •

edited

Loading

s77rt commented Mar 8, 2023 •

edited

Loading