[CP Staging] chore: attempt at fixing infinite loading on android#62827
Merged
marcaaron merged 11 commits intoExpensify:mainfrom May 27, 2025
callstack-internal:pac-guerreiro/bug/62688-test-drive-button-loads-infinitely-after-submitting-email
Conversation
Contributor
Author
|
@Beamanator @marcaaron @mountiny Can someone trigger an ADHOC build for this PR? I'm currently unable to test my solution with a local production build |
Contributor
|
triggered |
Contributor
|
Welp it looks like web may have succeeded, but it didn't get posted cuz a few of the actions failed 🤔 - https://github.com/Expensify/App/actions/runs/15261583814 |
Contributor
|
🚧 @Beamanator has triggered a test app build. You can view the workflow run here. |
Contributor
|
ok that should work lol 🙃 |
…ads-infinitely-after-submitting-email
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
@Beamanator testing now! |
Contributor
Author
|
@Beamanator the fix attempt doesn't work. I'll try another idea |
Contributor
|
darn ok thanks for the update! |
…ads-infinitely-after-submitting-email
Contributor
Author
|
@Beamanator can you trigger another adhoc build, please? 😄 |
Contributor
|
Sorry for the delay! Yes will do very soon!
…On Mon, May 26, 2025 at 5:06 PM Pedro Guerreiro ***@***.***> wrote:
*pac-guerreiro* left a comment (Expensify/App#62827)
<#62827 (comment)>
@Beamanator <https://github.com/Beamanator> can you trigger another adhoc
build, please? 😄
—
Reply to this email directly, view it on GitHub
<#62827 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA5UTP6LJ4KLE4SGRBZBIYD3AOM7DAVCNFSM6AAAAAB56LIDYSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMJQG42DANRWGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Contributor
|
🚧 @Beamanator has triggered a test app build. You can view the workflow run here. |
| import CONST from '@src/CONST'; | ||
|
|
||
| function getFileURL(asset: Asset, assetExtension: AssetExtension) { | ||
| const source = Image.resolveAssetSource(asset).uri; |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
51 tasks
This comment was marked as outdated.
This comment was marked as outdated.
…ltip not responding
…ads-infinitely-after-submitting-email
Contributor
Author
|
@Beamanator I was able to create a local release build using Screen.Recording.2025-05-27.at.06.44.15.movcc @fabioh8010 @linhvovan29546 I'm using react-native-fs copyFileRes and it worked! In the screen recording above you can observe that I was able to submit the receipt to Manager McTest 😄 |
Contributor
Author
|
I updated the PR checklist, added a proposal, testing steps and screen recording for android native |
Contributor
@pac-guerreiro Yeah, I previously tried that solution #62213 (comment), but it didn’t work. I might have passed the wrong value, so I ended up using a different approach. |
fabioh8010
reviewed
May 27, 2025
Contributor
fabioh8010
left a comment
fabioh8010
left a comment
There was a problem hiding this comment.
Looks really good! I will make some testing today too.
Contributor
There was a problem hiding this comment.
I suggest a few changes to getFile:
- Create a
types.tsfile to storegetFiletypes according to our guidelines. - Add comment explaining why we have to do this different code in Android.
| type SetTestReceipt = (asset: ImageSourcePropType, assetExtension: ReceiptExtension, onFileRead: OnFileRead) => void; | ||
| type AssetExtension = 'jpg' | 'png'; | ||
|
|
||
| type Asset = ImageSourcePropType; |
Contributor
There was a problem hiding this comment.
Suggested change
| type Asset = ImageSourcePropType; |
Doesn't seem necessary
|
|
||
| type SetTestReceipt = (asset: Asset, assetExtension: AssetExtension, onFileRead: OnFileRead, onFileError?: OnFileError) => void; | ||
|
|
||
| // eslint-disable-next-line import/prefer-default-export |
Contributor
There was a problem hiding this comment.
Suggested change
| // eslint-disable-next-line import/prefer-default-export |
Contributor
|
Are the iOS/Android adhoc builds crashing for anyone else? |
Contributor
Author
|
@Ollyws I'm unable to install iOS adhoc build on my physical iPhone 14 Plus. It says that the app integrity could not be verified. I guess this is due to the fact that my device is not registered |
Contributor
|
@pac-guerreiro is the Android adhoc working for you? My release build is taking forever. |
Contributor
Author
|
@Ollyws I'll give it a try and let you know |
Contributor
|
@Ollyws @pac-guerreiro Tested on my emulator and having crashes too (after I tried to sign in). Not sure what's happening but this PR isn't doing anything that could explain such crash. Screen.Recording.2025-05-27.at.17.08.59.mov |
Contributor
Author
|
@fabioh8010 damn 🫨 I confirm that the android app is crashing on my emulator also |
Contributor
Contributor
|
🚧 @Beamanator has triggered a test app build. You can view the workflow run here. |
…ads-infinitely-after-submitting-email
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
|
It's crashing for me on that build too. |
…ads-infinitely-after-submitting-email
marcaaron
approved these changes
May 27, 2025
Contributor
|
🚧 @marcaaron has triggered a test app build. You can view the workflow run here. |
Contributor
|
Going to do one more test on a few Android devices in browser stack to confirm there are no more crashes then will merge and CP this change. |
Contributor
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
Contributor
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp01_Android_Native.mp4Android: mWeb Chrome02_Android_Chrome.mp4iOS: HybridApp03_iOS_Native.mp4iOS: mWeb Safari04_iOS_Safari.mp4MacOS: Chrome / Safari05_MacOS_Chrome.mp4MacOS: Desktop06_MacOS_Desktop.mp4 |
marcaaron
changed the title
Contributor
|
Tested this via BrowserStack and was able to get through the flow 🎉 |
OSBotify
pushed a commit
that referenced
this pull request
May 27, 2025
…2688-test-drive-button-loads-infinitely-after-submitting-email chore: attempt at fixing infinite loading on android (cherry picked from commit 5cf5257) (cherry-picked to staging by marcaaron)
Contributor
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Contributor
|
🚀 Cherry-picked to staging by https://github.com/marcaaron in version: 9.1.51-10 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
8 tasks
8 tasks
Contributor
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.1.52-0 🚀
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
Fixed Issues
$#62688
PROPOSAL:
The cause for the infinite loading state on Android is due to bad error handling while reading test receipt asset file.
During our test we detected that, on release android builds, the app is unable to open the test receipt file due to a malformed url. This was not detected on debug android builds, since the test receipt file would be fetched from metro bundler. But on release android builds, the image assets are stored in
res/drawablefolder.I tried using
react-native-blob-utilasset method but it didn't work.I also tried using
file://protocol but it's not allowed.I was able to fix this using
react-native-fscopyFileRes method on release android builds, to copy the test receipt file fromres/drawableandroid folder to our cache folder under a new filename.Tests
Manage my team's expensesOffline tests
QA Steps
Manage my team's expensesPR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2025-05-27.at.06.44.15.mov
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop