Skip to content
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

[$500] IOU - After an IOU SmartScan creation error, the file is not downloaded #31845

Closed
6 tasks done
kbecciv opened this issue Nov 24, 2023 · 11 comments
Closed
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kbecciv
Copy link

kbecciv commented Nov 24, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 1.4.3.2
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Issue found when executing PR #29790

Action Performed:

  1. Open New Expensify app
  2. Navigate to any conversation
  3. Click + -> Request Money -> Scan
  4. Download the file from the example
  5. Complete the creation of the IOU
  6. Open the Money Request report chat
  7. Click on "Download the file" after the IOU creation error appears

Expected Result:

After the error of creating an IOU SmartScan file(check), the user should be able to download the file by clicking on "Download the file".

Actual Result:

After IOU SmartScan creation error, the file (receipt) is not downloaded, nothing happens when clicking on "Download the file".

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6288996_1700777171545.Recording_PR_29790_Desktop.1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01020d12092d3ed7d9
  • Upwork Job ID: 1728098006096576512
  • Last Price Increase: 2023-11-24
@kbecciv kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 24, 2023
Copy link

melvin-bot bot commented Nov 24, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01020d12092d3ed7d9

@melvin-bot melvin-bot bot changed the title IOU - After an IOU SmartScan creation error, the file is not downloaded [$500] IOU - After an IOU SmartScan creation error, the file is not downloaded Nov 24, 2023
Copy link

melvin-bot bot commented Nov 24, 2023

Triggered auto assignment to @garrettmknight (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 24, 2023
Copy link

melvin-bot bot commented Nov 24, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

Copy link

melvin-bot bot commented Nov 24, 2023

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External)

@shubham1206agra
Copy link
Contributor

shubham1206agra commented Nov 24, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue

After an IOU SmartScan creation error, the file is not downloaded

What is the root cause of that problem?

This is due to a missing filename (making it undefined) in fileDownload, which throws the error.

error TypeError: Cannot read properties of undefined (reading 'trim')
    at splitExtensionFromFileName (FileUtils.ts:103:1)
    at Module.appendTimeToFileName (FileUtils.ts:119:1)
    at eval (index.ts:33:35)

fileDownload didn't get the correct filename due to a bug in getReceiptError as this function expects the filename to be present in the receipt object, which is incorrect as we can check with the type.

This confusion of type will go away with typescript

What changes do you think we should make in order to solve the problem?

Add argument for filename in getReceiptError function

function getReceiptError(receipt, filename) {
    return _.isEmpty(receipt)
        ? ErrorUtils.getMicroSecondOnyxError('iou.error.genericCreateFailureMessage')
        : ErrorUtils.getMicroSecondOnyxErrorObject({error: CONST.IOU.RECEIPT_ERROR, source: receipt.source, filename});
}

What alternative solutions did you explore? (Optional)

@tienifr
Copy link
Contributor

tienifr commented Nov 24, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

After IOU SmartScan creation error, the file (receipt) is not downloaded, nothing happens when clicking on "Download the file".

What is the root cause of that problem?

  1. We're not setting the receipt.name to transaction receipt here. This is supposed to be used as the file name in the transaction.
  2. In here, we're not using the correct field of receipt, the correct field is name instead of filename

What changes do you think we should make in order to solve the problem?

  1. Add to here
receiptObject.name = receipt.name;
  1. Change filename to name here
filename: receipt.name

The split bill flow can be fixed similarly.

There's another issue, which is when we reload the page, we'll still see the Download the file button but clicking on it will open a blob with error. This is because blob cannot be persisted between page reload for security reason. If we want to fix this here, we need to check if the blob is still valid, if not do not show the Download the file button and the message can be adjusted a bit. Or we can still show the button but show a nice error dialog explaining the issue when the user clicks on it.

What alternative solutions did you explore? (Optional)

A

@shubham1206agra
Copy link
Contributor

For reference
This is the receipt type

type Receipt = {
    receiptID?: number;
    source?: string;
    state?: ValueOf<typeof CONST.IOU.RECEIPT_STATE>;
};

@tienifr
Copy link
Contributor

tienifr commented Nov 25, 2023

The filename in fact comes from the receipt.name as can be seen here, so the name does exist in the receipt, perhaps we can modify the Receipt type to correctly reflect that.

@shubham1206agra
Copy link
Contributor

Nope, I also checked from the backend. This is the structure I got. The confusion about receipt will be resolved when IOU.js gets migrated to TS.

{
    "amount": 0,
    "billable": false,
    "cardID": 17621655,
    "category": "",
    "comment": {
        "comment": ""
    },
    "created": "2023-11-25",
    "currency": "INR",
    "filename": "w_2396d6280fa6bd05322c143e2810e745fe4e5d5f.jpg",
    "hasEReceipt": false,
    "merchant": "(none)",
    "modifiedAmount": 0,
    "modifiedCreated": "",
    "modifiedCurrency": "",
    "modifiedMerchant": "",
    "originalAmount": 0,
    "originalCurrency": "",
    "parentTransactionID": "",
    "receipt": {
        "receiptID": 675782569,
        "source": "https://staging.expensify.com/receipts/w_2396d6280fa6bd05322c143e2810e745fe4e5d5f.jpg",
        "state": "SCANREADY"
    },
    "reimbursable": true,
    "reportID": "2000492459104116",
    "status": "Posted",
    "tag": "",
    "transactionID": "7229659494596007058"
}

@situchan
Copy link
Contributor

The offending PR will be reverted here

@melvin-bot melvin-bot bot added the Overdue label Nov 27, 2023
@garrettmknight
Copy link
Contributor

Looks like we're going to apply a fix for the separate issue the PR introduced instead of reverting the PR. We do have another issue open for this here #28884 so I'm going to close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

6 participants