Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
4.03k stars 3.03k forks source link

Remove hybrid app check from getEnvironment #56401

Closed jnowakow closed 5 days ago

jnowakow commented 1 week ago

@staszekscp

Explanation of Change

Earlier this check was necessary because there was no way to differentiate between staging and production. Now it's can be removed

Fixed Issues

$ https://github.com/Expensify/App/issues/52199

Tests

  1. Build app in various environments (dev, staging, prod)
  2. Verify that on all environments app connects to right API
  3. Verify that app has environment corresponding badge image

Offline tests

Same as in Tests but requests will fail

QA Steps

PR Author Checklist

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop
melvin-bot[bot] commented 1 week ago

@nikkiwines Please 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]

jnowakow commented 1 week ago

cc @AndrewGable

AndrewGable commented 1 week ago

Tests and QA section could be beefed up here 💪

jnowakow commented 1 week ago

@AndrewGable done 🫡 @staszekscp said that it was because there was no way to differentiate between staging and production. I'm not aware of what changed in the meantime so I'm not sure how to fully describe testing section. I built app locally with different buildTypes and variables in .env and it used different urls and had different badges depending on environment. Would be good if someone with deeper insight could test it before merging.

AndrewGable commented 1 week ago

@jnowakow - We only build the Android and iOS apps once for staging and production. We build the iOS and Android apps with production env and then send it to the staging audience. Then once we want to deploy to production we just move the latest staging version to production audience.

For web and desktop we build it twice, on staging we build with the staging env. On production we build again but using the production env.

github-actions[bot] commented 5 days ago

🚧 @AndrewGable has triggered a test hybrid app build. You can view the workflow run here.

github-actions[bot] commented 5 days ago
:test_tube::test_tube: Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! :test_tube::test_tube: Android :robot: iOS :apple:
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/56401/index.html https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/56401/index.html
Android iOS
Desktop :computer: Web :spider_web:
N/A N/A
N/A N/A

:eyes: View the workflow run that generated this build :eyes:

AndrewGable commented 5 days ago

Confirmed via adhoc 👍

share_8578038298221126279

AndrewGable commented 5 days ago

Reviewer Checklist

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop
OSBotify commented 5 days ago

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

github-actions[bot] commented 5 days ago

🚀 Deployed to staging by https://github.com/AndrewGable in version: 9.0.97-0 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅
🤖🔄 android HybridApp 🤖🔄 success ✅
🍎🔄 iOS HybridApp 🍎🔄 success ✅
izarutskaya commented 4 days ago

@jnowakow PR is failing on Android, iOS apps with https://github.com/Expensify/App/issues/56719 Pending validation on Web, mWeb an Desktop

https://platform.applause.com/services/links/v1/external/7adf738a0d11b914904d71b2bbef4f60f85dffcade55fcc54c9cb0da11ac0b30

Julesssss commented 4 days ago

I'm not sure how exactly, but this change caused blockers related to iOS linking, app deep links, ect.

When re-raising this case please add tests for these cases:

github-actions[bot] commented 4 days ago

🚀 Deployed to production by https://github.com/Julesssss in version: 9.0.97-1 🚀

platform result
🤖 android 🤖 true ❌
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅
🤖🔄 android HybridApp 🤖🔄 failure ❌
🍎🔄 iOS HybridApp 🍎🔄 failure ❌