Allow failed image/receipt uploads to be retried #54551

nyomanjyotisa · 2024-12-25T07:22:56Z

Explanation of Change

Fixed Issues

$ #50507
PROPOSAL: #50507 (comment)

Tests

Same as QA Steps

Verify that no errors appear in the JS console

Offline tests

Same as QA Steps

QA Steps

Test 1 (Replace Receipt):

Create an expense to a workspace or another user
Delete the receipt image & upload a new image for the receipt
Disrupt the upload by losing the internet connection or using another method
Verify that the receipt upload failed and display the error message: "There was an error uploading your receipt. Please try again or save the receipt to upload later."
Dismiss the receipt
Verify that the confirmation modal appears with the text: "Heads up! Dismissing this error will remove your uploaded receipt entirely. Are you sure?"
Click on "Dismiss"
Upload a new image for the receipt and repeat steps 3-4
Click on "save the file"
Verify that the image file is downloaded and the error message dismissed
Repeat steps 8
Click on "try again"
Verify that the image uploads successfully.

Test 2 (Split expense):

Split an expense to a workspace
Disrupt the upload by losing the internet connection or using another method
Verify that the receipt upload failed and display the error message: "There was an error uploading your receipt. Please try again or save the receipt to upload later."
Dismiss the receipt
Verify that the confirmation modal appears with the text: "Heads up! Dismissing this error will remove your uploaded receipt entirely. Are you sure?"
Click on "Dismiss"
Repeat steps 1-3
Click on "save the file"
Verify that the image file is downloaded and the error message dismissed
Repeat steps 1-3
Click on "try again"
Verify that the receipt uploads successfully.

Test 3 (Track expense):

Track an expense to a self DM
Disrupt the upload by losing the internet connection or using another method
Verify that the receipt upload failed and display the error message: "There was an error uploading your receipt. Please try again or save the receipt to upload later."
Dismiss the receipt
Verify that the confirmation modal appears with the text: "Heads up! Dismissing this error will remove your uploaded receipt entirely. Are you sure?"
Click on "Dismiss"
Repeat steps 1-3
Click on "save the file"
Verify that the image file is downloaded and the error message dismissed
Repeat steps 1-3
Click on "try again"
Verify that the receipt uploads successfully.

Test 4 (Request Money - Scan):

Create an expense to a workspace with a Receipt Scan
Disrupt the upload by losing the internet connection or using another method
Verify that the receipt upload failed and display the error message: "There was an error uploading your receipt. Please try again or save the receipt to upload later."
Dismiss the receipt
Verify that the confirmation modal appears with the text: "Heads up! Dismissing this error will remove your uploaded receipt entirely. Are you sure?"
Click on "Dismiss"
Repeat steps 1-3
Click on "save the file"
Verify that the image file is downloaded and the error message dismissed
Repeat steps 1-3
Click on "try again"
Verify that the receipt uploads successfully.

Test 5 (Request Money - Manual):

Manually create an expense to a workspace
Disrupt the upload by losing the internet connection or using another method
Verify that the receipt upload failed and display the generic error message: "Unexpected error submitting this expense. Please try again later."

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android-Native.mp4

Android: mWeb Chrome

Android-mWeb.Chrome.mp4

iOS: Native

iOS-Native.mp4

iOS: mWeb Safari

iOS-mWeb.Safari.mp4

MacOS: Chrome / Safari

MacOS-Chrome.mp4

MacOS: Desktop

MacOS-Desktop.mp4

nyomanjyotisa · 2024-12-27T00:42:46Z

After checking again, there is a new issue on Android Native where the "Try Again" feature causes the receipt to appear uploaded in the app but fails during the actual upload process. This likely happens because the receipt is currently stored in the Downloads directory, and native Android cannot fetch images stored outside the app's private sandbox.

I think we need to explore an alternative approach for fetching images on Android. Still looking for a possible fix
This issue did not happen on the old PR because the change merged after I raised the PR

cc @dukenv0307 @sakluger

Android-Native.mp4

dukenv0307 · 2024-12-31T03:40:50Z

@nyomanjyotisa Have you found the solution?

nyomanjyotisa · 2025-01-02T05:53:48Z

Yes, the issue on Android Native has been resolved

melvin-bot · 2025-01-02T06:00:21Z

@dukenv0307 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]

dukenv0307 · 2025-01-03T07:46:48Z

@nyomanjyotisa What about this place:

App/src/libs/actions/IOU.ts

Line 1533 in 1cfcfa4

    
           errors: getReceiptError(transaction.receipt, transaction.filename || transaction.receipt?.filename, false, errorKey),

? Do we need to add action and retryParams?

dukenv0307 · 2025-01-03T07:47:58Z

src/libs/actions/IOU.ts

+    filename?: string,
+    isScanRequest = true,
+    errorKey?: number,
+    action?: string,


We should defined the action enum here

dukenv0307 · 2025-01-03T08:26:51Z

src/libs/actions/IOU.ts

    return isEmptyObject(receipt) || !isScanRequest
        ? ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericCreateFailureMessage', errorKey)
-        : ErrorUtils.getMicroSecondOnyxErrorObject({error: CONST.IOU.RECEIPT_ERROR, source: receipt.source?.toString() ?? '', filename: filename ?? ''}, errorKey);
+        : ErrorUtils.getMicroSecondOnyxErrorObject(


In case users manually request money within receipt, the isScanRequest will be false, so the receipt error will not added to failureData. IMO, isEmptyObject(receipt) is enough, but not sure why the author add !isScanRequest check. Can you check this case?

I agree, isEmptyObject(receipt) should be enough in this case. I’m not entirely sure why that additional check was included either. If I remove the !isScanRequest condition, this line will cause the generic error to display, resulting in a duplicate error message.

How can you show these messages? I can't reproduce on my side

Screen.Recording.2025-01-06.at.11.18.41.mov

I think this line doesn't relate to isScanRequest. No matter what isScanRequest is, this error message is still added.

@nyomanjyotisa But I think it's expected not to show receipt error in case manual request. So we should update the test steps.

For manual request: Show the generic message
For receipt request: Show the image error message

dukenv0307 · 2025-01-03T08:28:35Z

src/libs/actions/IOU.ts

@@ -318,6 +321,56 @@ Onyx.connect({
    },
 });

+type StartSplitBilActionParams = {


I think we will refactor the params in this issue so we shouldn't create the new type in this PR.

If that’s the case, how would you suggest handling retryParams in getReceiptError without introducing a new parameter like StartSplitBillActionParams? I believe the parameters haven’t been fully addressed in the mentioned issue yet right?

Actually, you didn't introdude the new StartSplitBilActionParams, you just moved it to other place. So it's fine

dukenv0307 · 2025-01-03T08:30:26Z

I tested the retry and download feature on web and safari, they worked well but need to wait for @nyomanjyotisa to resolve the above comments before completing the tests on other platforms

nyomanjyotisa · 2025-01-06T01:08:08Z

Do we need to add action and retryParams?

Correct me if I'm wrong, but I don't think action and retryParams are necessary there since the invoice doesn’t include a receipt

dukenv0307 · 2025-01-06T04:02:27Z

Correct me if I'm wrong, but I don't think action and retryParams are necessary there since the invoice doesn’t include a receipt

Yah, so I think we don't need to add retryParams here

dukenv0307 · 2025-01-06T04:53:40Z

@nyomanjyotisa Some tests are failed, can you pls check?

nyomanjyotisa · 2025-01-07T03:07:28Z

Sorry for the late response. The failed test has been solved @dukenv0307

dukenv0307 · 2025-01-08T03:12:54Z

Reviewing...

dukenv0307 · 2025-01-08T16:05:21Z

src/components/DotIndicatorMessage.tsx

+                                            ? (JSON.parse(message.retryParams) as IOU.ReplaceReceipt | IOU.StartSplitBilActionParams | IOU.TrackExpense | IOU.RequestMoneyInformation)
+                                            : message.retryParams;
+
+                                    switch (message.action) {


use ACTION_PARAMS const

dukenv0307 · 2025-01-08T16:06:05Z

src/components/DotIndicatorMessage.tsx

+                        <Text style={[StyleUtils.getDotIndicatorTextStyles(isErrorMessage)]}>{Localize.translateLocal('iou.error.receiptFailureMessage')}</Text>
+                        <TextLink
+                            style={[StyleUtils.getDotIndicatorTextStyles(), styles.link]}
+                            onPress={() => {


Can you pls define the new function for it (instead of anonymous func) ?

dukenv0307 · 2025-01-10T11:10:46Z

@nyomanjyotisa Pls let me know when it's ready for review. Thanks

dukenv0307 · 2025-01-13T10:58:05Z

@nyomanjyotisa friendly bump

nyomanjyotisa · 2025-01-14T04:07:39Z

The PR has been updated. Could you please review it again, @dukenv0307?

dukenv0307 · 2025-01-15T04:28:18Z

For handleRetryPress function, we should define it in libs. Then keep the src/components/DotIndicatorMessage.tsx as before

dukenv0307 · 2025-01-15T04:30:10Z

handleFileRetry are duplicated.

dukenv0307 · 2025-01-15T04:31:50Z

src/pages/iou/request/step/IOURequestStepDistance.tsx

-                        undefined,
-                        undefined,
-                        undefined,
+                        policyTagList: undefined,


remove undefined value

dukenv0307 · 2025-01-15T04:34:46Z

src/pages/iou/request/step/IOURequestStepDistance.tsx

-                        translate('iou.fieldPending'),
-                        currentUserPersonalDetails.login,
-                        currentUserPersonalDetails.accountID,
+                        amount: 0,


we should use default value in trackExpense, then remove the unnecessary value here

dukenv0307 · 2025-01-15T04:41:26Z

Pls refactor the trackExpense function call

dukenv0307 · 2025-01-17T02:45:33Z

@nyomanjyotisa Any updates?

dukenv0307 · 2025-01-20T03:50:07Z

@nyomanjyotisa bump

nyomanjyotisa · 2025-01-20T04:21:55Z

Working on this today

nyomanjyotisa · 2025-01-20T15:35:29Z

PR updated @dukenv0307

dukenv0307 · 2025-01-21T09:28:43Z

src/components/DotIndicatorMessage.tsx

 import useStyleUtils from '@hooks/useStyleUtils';
 import useTheme from '@hooks/useTheme';
 import useThemeStyles from '@hooks/useThemeStyles';
 import {isReceiptError} from '@libs/ErrorUtils';
 import fileDownload from '@libs/fileDownload';
-import * as Localize from '@libs/Localize';
+import {translateLocal} from '@libs/Localize';
+import handleRetryPress from '@libs/ReceiptUploadRetryHandler/index';


Suggested change

import handleRetryPress from '@libs/ReceiptUploadRetryHandler/index';

import handleRetryPress from '@libs/ReceiptUploadRetryHandler';

nyomanjyotisa added 3 commits December 25, 2024 12:07

Allow failed image/receipt uploads to be retried

3be9e08

Allow failed image/receipt uploads to be retried

3b51763

ESLint fix

9a666b5

nyomanjyotisa mentioned this pull request Dec 26, 2024

[$500] Allow failed image/receipt uploads to be retried #50507

Open

6 tasks

test fix

f1f05e1

Android Native Fix

37850db

Allow failed image/receipt uploads to be retried

7980a1d

nyomanjyotisa marked this pull request as ready for review January 2, 2025 06:00

nyomanjyotisa requested a review from a team as a code owner January 2, 2025 06:00

melvin-bot bot requested review from dukenv0307 and removed request for a team January 2, 2025 06:00

dukenv0307 reviewed Jan 3, 2025

View reviewed changes

Add action enum

624ec5f

nyomanjyotisa added 2 commits January 7, 2025 10:13

Merge branch 'main' into issue-50507-2

4d0356d

ESLint fix

f0f1889

dukenv0307 reviewed Jan 8, 2025

View reviewed changes

nyomanjyotisa added 3 commits January 9, 2025 19:52

ESLint fix

13cfcb1

Separate DotIndicatorMessage

5ac07e2

Update copy

11a6718

Update translation

802e273

nyomanjyotisa added 4 commits January 14, 2025 11:01

Merge branch 'main' into issue-50507-2

3f65481

IOUTest fix

b89f971

IOUTest fix

f5135bb

IOUTest property shorthand

a03684d

dukenv0307 reviewed Jan 15, 2025

View reviewed changes

DotIndicatorMessage adjustment

2a70a0c

nyomanjyotisa added 7 commits January 20, 2025 12:59

Merge branch 'main' into issue-50507-2

26af42a

Merge branch 'main' into issue-50507-2

3e5e2c6

ESLint fix

ab6b967

Update trackExpense function call

8390821

Merge branch 'main' into issue-50507-2

fef81e0

ESLint fix

9e27e13

Prettier

349aaa7

ESLint fix

3ae692a

dukenv0307 reviewed Jan 21, 2025

View reviewed changes

	import handleRetryPress from '@libs/ReceiptUploadRetryHandler/index';
	import handleRetryPress from '@libs/ReceiptUploadRetryHandler';

Allow failed image/receipt uploads to be retried #54551

Are you sure you want to change the base?

Allow failed image/receipt uploads to be retried #54551

Conversation

nyomanjyotisa commented Dec 25, 2024 • edited Loading

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

nyomanjyotisa commented Dec 27, 2024 • edited Loading

dukenv0307 commented Dec 31, 2024

nyomanjyotisa commented Jan 2, 2025

melvin-bot bot commented Jan 2, 2025

dukenv0307 commented Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nyomanjyotisa Jan 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dukenv0307 Jan 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dukenv0307 commented Jan 3, 2025

nyomanjyotisa commented Jan 6, 2025

dukenv0307 commented Jan 6, 2025

dukenv0307 commented Jan 6, 2025

nyomanjyotisa commented Jan 7, 2025

dukenv0307 commented Jan 8, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dukenv0307 commented Jan 10, 2025

dukenv0307 commented Jan 13, 2025

nyomanjyotisa commented Jan 14, 2025

dukenv0307 commented Jan 15, 2025 • edited Loading

dukenv0307 commented Jan 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dukenv0307 commented Jan 15, 2025

dukenv0307 commented Jan 17, 2025

dukenv0307 commented Jan 20, 2025

nyomanjyotisa commented Jan 20, 2025

nyomanjyotisa commented Jan 20, 2025

Choose a reason for hiding this comment

nyomanjyotisa commented Dec 25, 2024 •

edited

Loading

nyomanjyotisa commented Dec 27, 2024 •

edited

Loading

dukenv0307 commented Jan 3, 2025 •

edited

Loading

nyomanjyotisa Jan 6, 2025 •

edited

Loading

dukenv0307 Jan 6, 2025 •

edited

Loading

dukenv0307 commented Jan 15, 2025 •

edited

Loading