-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[HOLD for payment 2024-11-05] [$250] IOU - mWeb - User trying to upload receipt by quickly tapping capture button shows error #50083
Comments
Triggered auto assignment to @marcochavezf ( |
Triggered auto assignment to @Christinadobrzyn ( |
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
|
This is a frontend issue, making it external to get more 👀 |
Job added to Upwork: https://www.upwork.com/jobs/~021841557091803230884 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @fedirjh ( |
I cannot reproduce this error consistently in staging. Also, I don't think this specific behavior on mweb should be considered a blocker, but it still needs a fix |
Edited by proposal-police: This proposal was edited at 2024-10-03 09:00:50 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Quickly tapping the shutter results in an error or crash. What is the root cause of that problem?The root causes for web and native platforms are different. Android mWeb & iOS mWeb
Android Native & iOS Native
After one operation completes, the page navigates back and the camera closes, so the remaining operations fail. App/src/pages/iou/request/step/IOURequestStepScan/index.native.tsx Lines 454 to 459 in efa3274
App/src/pages/iou/request/step/IOURequestStepScan/index.native.tsx Lines 465 to 470 in efa3274
What changes do you think we should make in order to solve the problem?Android mWeb & iOS mWeb
const [isReady, setIsReady] = useState(false);
setIsReady(true);
style={[styles.alignItemsCenter, !isReady && styles.opacity0]}
disabled={!isReady}
Android Native & iOS Native
App/src/pages/iou/request/step/IOURequestStepScan/index.native.tsx Lines 454 to 455 in efa3274
}, () => {
setDidCapturePhoto(false);
showCameraAlert();
Log.warn('Error reading photo');
}); What alternative solutions did you explore? (Optional)Android mWeb & iOS mWebOn the frontend, we can add a
if (imageBase64 === null) {
return;
} |
Not overdue waiting for C+ review and/or more proposals |
@fedirjh can you provide an update on this? Thanks! |
@QichenZhu Proposal looks good to me. The root cause analysis and solution seem accurate. Let's move forward with the alternative solution. It is straightforward and does not require any design changes. 🎀 👀 🎀 C+ reviewed |
@fedirjh This is marked as Android mWeb only, but I got an error pop-up when testing on Android Native, and the app crashed on iOS Native.
Do you think it should be handled in a separate issue? |
I think we can just include it in this issue if it's the same issue on a different device. Does that sound good, @fedirjh? |
I think the issue on native platforms is not related to the web issue. Previously, we had an issue #36520, where double tapping the shutter caused an error. It was fixed by #38320 for the new money request workflow but not for the edit request workflow, so it appears here now. The root cause is explained here: #36520 (comment). |
ProposalAdded the root cause and solution for the native platform issue. |
@fedirjh can you please review and let us know what you think is best, should these be fixed together or separate? |
@marcochavezf @fedirjh @Christinadobrzyn @QichenZhu this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
@marcochavezf, @fedirjh, @Christinadobrzyn, @QichenZhu Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@QichenZhu thanks for bringing this up, the solution for native LGTM. We can address any other issue (if any) in the PR |
Just checking in on this, I think we're working on a PR, is that right @QichenZhu? |
@Christinadobrzyn The PR #50770 has been merged and deployed to staging. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.54-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-11-05. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
Payouts due:
@fedirjh do we need a regression test for this? |
Yes. Regression Test Proposal
|
Awesome! payment day is today. Payment Summary. Regression test https://github.com/Expensify/Expensify/issues/442076 @fedirjh can you please request payment through ND? |
DMd @fedirjh to request payment. Going to close this as complete since that's all that's outstanding. |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: v9.0.43-1
Reproducible in staging?: Y
Reproducible in production?: N
Issue reported by: Applause Internal Team
Action Performed:
Expected Result:
User trying to upload receipt by quickly tapping capture button must able to upload receipt.
Actual Result:
User trying to upload receipt by quickly tapping capture button shows error "The receipt didn't upload.Download the file or dismiss this error and lose it."
Workaround:
Unknown
Platforms:
Screenshots/Videos
Bug6622046_1727886169840.Screenrecorder-2024-10-02-21-46-43-70_compress_1.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @ChristinadobrzynThe text was updated successfully, but these errors were encountered: