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

Unable to upload HEIF image as money request (Web) #47078

Open
1 of 6 tasks
m-natarajan opened this issue Aug 8, 2024 · 88 comments
Open
1 of 6 tasks

Unable to upload HEIF image as money request (Web) #47078

m-natarajan opened this issue Aug 8, 2024 · 88 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented Aug 8, 2024

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: 9.0.18-1
Reproducible in staging?: Y
Reproducible in production?: N
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: @Beamanator
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1723093506634689

Action Performed:

  1. Take picture on iPhone 12 (HEIF image)
  2. Try to request money & upload by scanning receipt IN WEB

Expected Result:

Image should upload & scan

Actual Result:

Saw error message "Invalid file type"

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

RPReplay_Final1723093247.MP4

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @Beamanator
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0116d43a98c70752e7
  • Upwork Job ID: 1826245037583994789
  • Last Price Increase: 2024-08-21
  • Automatic offers:
    • nyomanjyotisa | Contributor | 103881407
@m-natarajan m-natarajan added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. DeployBlocker Indicates it should block deploying the API labels Aug 8, 2024
Copy link

melvin-bot bot commented Aug 8, 2024

Triggered auto assignment to @kevinksullivan (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

Copy link

melvin-bot bot commented Aug 8, 2024

Triggered auto assignment to @johnmlee101 (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

Copy link
Contributor

github-actions bot commented Aug 8, 2024

👋 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:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@nyomanjyotisa
Copy link
Contributor

nyomanjyotisa commented Aug 8, 2024

Edited by proposal-police: This proposal was edited at 2024-09-05 12:18:12 UTC.

Proposal

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

Unable to upload HEIF image as money request

What is the root cause of that problem?

heif is not in the list of ALLOWED_RECEIPT_EXTENSIONS

ALLOWED_RECEIPT_EXTENSIONS: ['jpg', 'jpeg', 'gif', 'png', 'pdf', 'htm', 'html', 'text', 'rtf', 'doc', 'tif', 'tiff', 'msword', 'zip', 'xml', 'message'],

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

Add heif and heic to ALLOWED_RECEIPT_EXTENSIONS

We also need to convert the heif/heic image to jpeg first before displaying and uploading the receipt image to BE

We cant use expo-image-manipulator like we use here on web so I propose we use heic-to for the converter

Create new function called convertReceipt

    const convertReceipt = async (originalFile: FileObject, isPdfValidated?: boolean) => {
        if (originalFile?.type?.startsWith('image')) {
            if (!originalFile.uri) {
                console.error('File URI is undefined');
                return;
            }

            try {
                const response = await fetch(originalFile.uri);
                const blob = await response.blob();

                const fileFromBlob = new File([blob], originalFile.name || 'temp-file', {
                    type: blob.type,
                });

                const isHEIC = await isHeic(fileFromBlob);

                if (isHEIC) {
                    setIsLoadingReceipt(true);

                    try {
                        const convertedBlob = await heicTo({
                            blob,
                            type: 'image/jpeg',
                        });

                        const fileName = originalFile.name ? originalFile.name.replace(/\.heic$/i, '.jpg') : 'converted-image.jpg';

                        const jpegFile = new File([convertedBlob], fileName, {type: 'image/jpeg'});

                        setReceiptAndNavigate(jpegFile, isPdfValidated);
                    } catch (err) {
                        console.error('Error converting HEIC to JPEG:', err);
                    } finally {
                        setIsLoadingReceipt(false);
                    }
                } else {
                    setReceiptAndNavigate(originalFile, isPdfValidated);
                }
            } catch (err) {
                console.error('Error processing the file:', err);
            }
        } else {
            setReceiptAndNavigate(originalFile, isPdfValidated);
        }
    };

Change setReceiptAndNavigate usage to convertReceipt here here and here

On the other side, the heic-to is incompatible with native platforms as it relies on Workers, which is not supported when using Hermes. The user cannot upload the receipt if it was uploaded from 'Choose File Option'

New-Expensify.mp4

To address this, we can create convertReceipt function using the expo-image-manipulator to convert the receipt on native platforms on index.native.tsx

What alternative solutions did you explore? (Optional)

@Beamanator
Copy link
Contributor

Being discussed a bit in slack - https://expensify.slack.com/archives/C01GTK53T8Q/p1723093823149789

@Beamanator Beamanator removed DeployBlocker Indicates it should block deploying the API DeployBlockerCash This issue or pull request should block deployment labels Aug 9, 2024
@Beamanator
Copy link
Contributor

Working well in staging! marking NAB (probably shouldn't close though, since i think we still can't upload HEIF on web)

@Beamanator Beamanator added Daily KSv2 and removed Hourly KSv2 labels Aug 9, 2024
@Beamanator
Copy link
Contributor

@johnmlee101 feel free to assign to me if you want, but totes up to you

@melvin-bot melvin-bot bot added Overdue Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Aug 12, 2024
@melvin-bot melvin-bot bot changed the title Unable to upload HEIF image as money request [HOLD for payment 2024-08-19] Unable to upload HEIF image as money request Aug 12, 2024
@melvin-bot melvin-bot bot removed the Overdue label Aug 12, 2024
Copy link

melvin-bot bot commented Aug 12, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.18-10 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-08-19. 🎊

@nyomanjyotisa
Copy link
Contributor

As I mentioned in this comment, there are no issues on native platforms when users upload images directly from the gallery. However, if a receipt is uploaded using the 'Choose File' option, the image is not converted causing the upload to fail. To address this, I suggest using expo-image-manipulator to convert HEIC files on native platforms like on this index.native.ts. Let me know if there is something that I missed

@parasharrajat
Copy link
Member

Ok. Thanks. @Beamanator Do you approve using heic-to lib?

@garrettmknight garrettmknight moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Dec 3, 2024
@melvin-bot melvin-bot bot added the Overdue label Dec 6, 2024
@Beamanator
Copy link
Contributor

However, if a receipt is uploaded using the 'Choose File' option, the image is not converted causing the upload to fail.

What exactly fails with that upload? Like right now, uploading HEIC from iOS native works... Right? I assume we convert to jpg in the backend? Why doesn't that work with the new plan?

@melvin-bot melvin-bot bot removed the Overdue label Dec 7, 2024
@nyomanjyotisa
Copy link
Contributor

There is a convert HEIC to JPEG only on native here,

if (targetAsset?.type?.startsWith('image')) {
FileUtils.verifyFileFormat({fileUri: targetAssetUri, formatSignatures: CONST.HEIC_SIGNATURES})
.then((isHEIC) => {
// react-native-image-picker incorrectly changes file extension without transcoding the HEIC file, so we are doing it manually if we detect HEIC signature
if (isHEIC && targetAssetUri) {
manipulateAsync(targetAssetUri, [], {format: SaveFormat.JPEG})
.then((manipResult) => {
const uri = manipResult.uri;
const convertedAsset = {
uri,
name: uri
.substring(uri.lastIndexOf('/') + 1)
.split('?')
.at(0),
type: 'image/jpeg',
width: manipResult.width,
height: manipResult.height,
};
return resolve([convertedAsset]);
})
.catch((err) => reject(err));
} else {
return resolve(response.assets);
}
})
.catch((err) => reject(err));
} else {
return resolve(response.assets);
}

However, we can't use the same approach/same module (expo-image-manipulator) in the web environment because on the web, the URI format is a Blob, and expo-image-manipulator does not support Blob conversion
And for the 'Choose File' option, I believe the issue occurs because the uploaded image only converted on showImagePicker and not on showDocumentPicker

@melvin-bot melvin-bot bot added the Overdue label Dec 16, 2024
@Beamanator
Copy link
Contributor

Ok ok sorry for the delay here... @nyomanjyotisa since you're assigned here, can you please summarize where your latest proposal stands? If you & @parasharrajat agree we will need a new lib (https://github.com/hoppergee/heic-to) & it looks safe to use, I will have to bring it to slack again & get it approved before we can move forward

@melvin-bot melvin-bot bot removed the Overdue label Dec 17, 2024
@nyomanjyotisa
Copy link
Contributor

Proposal updated

@parasharrajat
Copy link
Member

parasharrajat commented Dec 19, 2024

Looks like that heic-to is not available on npm. It needs to be directly installed from github which can be issue on long run. But on the same time heic2Any lib is not up-to-date. There is no update since last year.

@Beamanator Technically, proposal looks good to me. I leave the decision to select the lib up to you.

@melvin-bot melvin-bot bot added the Overdue label Dec 26, 2024
@parasharrajat
Copy link
Member

@Beamanator Can you please review this Library with the help of team? Are we good with directly installing the package from the github repo?

@parasharrajat
Copy link
Member

Bump @Beamanator

@Beamanator
Copy link
Contributor

juuuuust got back from vacay, will get to this soon but probably not today

@melvin-bot melvin-bot bot removed the Overdue label Jan 7, 2025
@melvin-bot melvin-bot bot added the Overdue label Jan 15, 2025
@Beamanator
Copy link
Contributor

Sorryyyy - deployer this week so will be pretty busy 😢 will try to get back to this soon

@Beamanator
Copy link
Contributor

Ok created the new lib request issue: #55777

I will post in slack once we have a few answers:

  1. What will the effect be on the bundle size of our code?
  2. What will be the process of using this lib in our code? Do we need to build anything on our own devices before running it locally?

@nyomanjyotisa @parasharrajat please take a look and let me know if anything's missing.

@melvin-bot melvin-bot bot added the Overdue label Feb 5, 2025
@Beamanator
Copy link
Contributor

Bump @nyomanjyotisa @parasharrajat 🙏

@melvin-bot melvin-bot bot removed the Overdue label Feb 11, 2025
@parasharrajat
Copy link
Member

@Beamanator

  1. Approx 9 MB minified size https://bundlephobia.com/package/[email protected]
  2. We don't need to build anything, it's prebuilt. It depends on libheif which taking almost all of 9 MB.

@melvin-bot melvin-bot bot added the Overdue label Feb 19, 2025
@Beamanator
Copy link
Contributor

Thanks @parasharrajat ! 🙏

I will bring this to slack next week, as i'm going OOO for pretty much the rest of this week 👍

@melvin-bot melvin-bot bot removed the Overdue label Feb 19, 2025
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. Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
Status: Bugs and Follow Up Issues
Development

No branches or pull requests

8 participants