-
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
Toggle staging and production server api #10690
Toggle staging and production server api #10690
Conversation
Add Test steps & Screen recordings! |
bump @mdneyazahmad |
@Santhosh-Sellavel Updated! Let me know if the description is fine. Thanks. |
@mdneyazahmad Also, we could test in Desktop too. Refer below Screen.Recording.2022-09-03.at.11.30.09.PM.mov |
@Santhosh-Sellavel Updated! Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look promising, gonna wait for @Santhosh-Sellavel to do a full testing, but so far great job!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay, LGTM!
I will test it again once it is deployed, thanks!
@mountiny
|
all you @mountiny! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, great work! You might want to merge main into this branch to make the checklist action work. I am double checking how to fix that
Thanks @mdneyazahmad |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Per checklist, i am noting here, that this PR has not worked correctly to a full extent, where the toggle to staging API did not work in native apps and we had to fix it in this issue. We had to test this in staging better and it must have been tested internally with checking the logs since it is not easy to see where the API requests from native apps are going. |
Details
This PR tries to fix the toggle button on staging environment to select either staging (default) or production api server. This will help us catch bugs on server api before, we deploy that to production.
Note: This PR is not testable locally.
Fixed Issues
$ #10163
Test (Web & Desktop)
Settings -> Preference
Use staging server
switch in on by defaultNetwork
tab.Fetch/XHR
is of staging serverhttps://staging.expensify.com
orhttps://staging-secure.expensify.com
.Use staging server
(it should be off)Fetch/XHR
is of production server (without staging prefixes)https://www.expensify.com
orhttps://secure.expensify.com
.Tests (excluding Web & Desktop)
Settings -> Preference
Use staging server
switch in on by defaultPR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
filesSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
displayName
propertythis
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)PR Reviewer Checklist
The Contributor+ will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
).src/languages/*
filesSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
have been tested & I retested again)/** comment above it */
displayName
propertythis
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)QA Steps (Web & Desktop)
Settings -> Preference
Use staging server
switch in on by defaultNetwork
tab.Fetch/XHR
is of staging serverhttps://staging.expensify.com
orhttps://staging-secure.expensify.com
.Use staging server
(it should be off)Fetch/XHR
is of production server (without staging prefixes)https://www.expensify.com
orhttps://secure.expensify.com
.QA Steps (excluding Web & Desktop)
Settings -> Preference
Use staging server
switch in on by defaultScreenshots
Web
web.mp4
Mobile Web
mweb.mp4
Desktop
desktop.mp4
iOS
ios.mp4
Android
android.mp4