Improve receipt file detection#55321
Conversation
|
@thesahindia 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] |
|
cc @Pujan92 |
Reviewer Checklist
Screenshots/VideosAndroid: Nativefix2.webmAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2025-01-23.at.17.58.48.movMacOS: Desktop |
|
Plz resolve the conflicts. |
…ecks-for-receipt-file-detection
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #55240 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
@CyberAndrii @Pujan92 have we specifically tested this on hybridApp? I don't think @CyberAndrii has the ability to test on hybrid app but @Pujan92 you should be able to, can you confirm this builds and behaves correctly on iOS/Android hybrid app? |
…ecks-for-receipt-file-detection
|
Resolved conflicts. @Pujan92 bump on #55321 (comment) |
I haven't setup earlier the hybrid app locally, I need to go through the documentation to run it locally. Mostly it will be done and tested by tomorrow. |
|
@CyberAndrii can you fix the merge conflicts while we're waiting for @Pujan92 to test this with hybrid app? |
|
Update: I am facing an issue setting the hybrid app https://expensify.slack.com/archives/C02NK2DQWUX/p1738068384747799 |
…ecks-for-receipt-file-detection
|
Tests are failing due to unrelated changes. I see this on other recent PRs. |
|
@blimpich Looks fine for hybrid android app pa1.webm |
blimpich
left a comment
There was a problem hiding this comment.
Sweet! Lets do this! Right now merging to main is frozen 🥶, but we can try to merge this in tomorrow when hopefully the merge freeze is over.
@CyberAndrii do you know if we need to update with the latest main to fix the issues or just re-run to fix flakey tests?
|
Should be fixed by re-running according to #56057 (comment) |
|
@CyberAndrii dang, one more merge conflict. @CyberAndrii can you fix that and then we can merge? |
…ecks-for-receipt-file-detection
…ecks-for-receipt-file-detection
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/blimpich in version: 9.0.94-0 🚀
|
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.0.94-25 🚀
|
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.0.94-25 🚀
|
Explanation of Change
Follow up on #54628 to fix a missed case where the
receiptis not aFileobject on Android when tracking a Scan expense via the Quick action button.This PR updates the
receipt instanceof Blobcheck to allow receipts in the format{uri: 'path', ...}as required by React Native (native platforms only).Fixed Issues
$ #55240
PROPOSAL:
Tests
Same as QA Steps.
Offline tests
N/A
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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
55240.android.native.webm
Android: mWeb Chrome
55240.android.chrome.webm
iOS: Native
55240.ios.native.mp4
iOS: mWeb Safari
55240.ios.safari.mp4
MacOS: Chrome / Safari
55240.macos.chrome.mov
MacOS: Desktop
55240.macos.desktop.mov