Unable to upload HEIF image as money request (Web)

m-natarajan commented 3 months ago

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

Take picture on iPhone 12 (HEIF image)
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
- [x] MacOS: Chrome / Safari
- [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/5b71a34d-944c-4b17-bc36-ac2548d7238b

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @parasharrajat

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

melvin-bot[bot] commented 3 months ago

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.

melvin-bot[bot] commented 3 months ago

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

github-actions[bot] commented 3 months ago

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

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

nyomanjyotisa commented 3 months ago

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 https://github.com/Expensify/App/blob/d7b0aba10b580b7b8adebb3cdb2a8026f119dafa/src/CONST.ts#L122

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

add heif and heic to ALLOWED_RECEIPT_EXTENSIONS

we 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 heic2any for the converter

create new function convertReceipt

    const convertReceipt = (originalFile: FileObject, isPdfValidated?: boolean) => {
        if (originalFile?.type?.startsWith('image')) {
            FileUtils.verifyFileFormat({ fileUri: originalFile.uri, formatSignatures: CONST.HEIC_SIGNATURES })
                .then((isHEIC) => {                    
                    if (isHEIC && originalFile.uri) {
                        setIsLoadingReceipt(true);

                        fetch(originalFile.uri)
                            .then((response) => response.blob())
                            .then((blob) => heic2any({ blob, toType: 'image/jpeg' }))
                            .then((convertedBlob) => {
                                const blobToUse = Array.isArray(convertedBlob) ? convertedBlob[0] : convertedBlob;

                                const jpegFile = new File([blobToUse], originalFile.name.replace(/\.heic$/i, '.jpg'), { type: 'image/jpeg' });

                                setReceiptAndNavigate(jpegFile, isPdfValidated);
                            })
                            .catch((err) => {
                                setIsLoadingReceipt(false);
                            });
                    } else {
                        setReceiptAndNavigate(originalFile, isPdfValidated);
                    }
                })
                .catch((err) => {
                    console.error('Error verifying file format:', err);
                });
        } else {
            setReceiptAndNavigate(originalFile, isPdfValidated);
        }
    };

And change setReceiptAndNavigate usage to convertReceipt here here and here

RESULT

https://github.com/user-attachments/assets/e1578be1-1feb-4df8-b266-ee07f8d2eadc

What alternative solutions did you explore? (Optional)

Beamanator commented 3 months ago

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

Beamanator commented 3 months ago

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

Beamanator commented 3 months ago

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

melvin-bot[bot] commented 2 months ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.18-10 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

https://github.com/Expensify/App/pull/47139

If no regressions arise, payment will be issued on 2024-08-19. :confetti_ball:

melvin-bot[bot] commented 2 months ago

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:

[ ] [@Beamanator / @johnmlee101] The PR that introduced the bug has been identified. Link to the PR:
[ ] [@Beamanator / @johnmlee101] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[ ] [@Beamanator / @johnmlee101] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[ ] [@Beamanator / @johnmlee101] Determine if we should create a regression test for this bug.
[ ] [@Beamanator / @johnmlee101] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[ ] [@kevinksullivan] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Beamanator commented 2 months ago

Ok so HEIF image uploads should be working well on iOS now (i use this every day) but I believe it doesn't work in web... @BrtqKr since you worked on https://github.com/Expensify/App/pull/47139 and https://github.com/Expensify/App/pull/46025, are you interested in giving that a shot at a fix?

melvin-bot[bot] commented 2 months ago

Skipping the payment summary for this issue since all the assignees are employees or vendors. If this is incorrect, please manually add the payment summary SO.

Beamanator commented 2 months ago

OK ya the CP worked but we're keeping this open to hopefully fix HEIF uploads on ALL platforms

kevinksullivan commented 2 months ago

No payment necessary. Can we close this out @Beamanator ?

Beamanator commented 2 months ago

Can't close b/c we still can't upload HEIF images on web - @BrtqKr are you interested in taking that on? If not, we can open this up to be External

BrtqKr commented 2 months ago

@Beamanator let's pass it to the external, I've two tickets I'm either taking care of right now, or will be taking care soon

Beamanator commented 2 months ago

Giggidy, sounds good here!

melvin-bot[bot] commented 2 months ago

Job added to Upwork: https://www.upwork.com/jobs/~0116d43a98c70752e7

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

@Beamanator @johnmlee101 @kevinksullivan @parasharrajat 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!

parasharrajat commented 2 months ago

No proposals.

melvin-bot[bot] commented 2 months ago

@Beamanator, @johnmlee101, @kevinksullivan, @parasharrajat Eep! 4 days overdue now. Issues have feelings too...

parasharrajat commented 2 months ago

It looks like we just need @nyomanjyotisa's solution. @nyomanjyotisa Can you please share a video on how it works after the change?

melvin-bot[bot] commented 2 months ago

@Beamanator, @johnmlee101, @kevinksullivan, @parasharrajat Huh... This is 4 days overdue. Who can take care of this?

Beamanator commented 2 months ago

Bump @nyomanjyotisa 🙏

parasharrajat commented 2 months ago

@nyomanjyotisa Are you still interested in the issue?

nyomanjyotisa commented 2 months ago

@Beamanator @parasharrajat Sorry for the delay, here I attach the video

https://github.com/user-attachments/assets/89424dc6-4f51-4d51-858b-d61ead214d94 The image can be uploaded but there is no preview and I got an unexpected error when uploading the image with this response

{
    "code": 666,
    "jsonCode": 666,
    "type": "Expensify\\Libs\\Error\\ExpError",
    "UUID": "49c8f797-ecaa-4e5f-a63a-a84de1e26534",
    "message": "Unsupported PDF format",
    "title": "",
    "data": {
        "filename": "\/tmp\/tmpFile9Mn0OB",
        "message": "Unsupported file type: image\/heic",
        "responseCode": 2001
    },
    "htmlMessage": "",
    "onyxData": [],
    "requestID": "8be54d0a6d825fe1-SIN"
}

I think the image should converted to JPEG or PNG format by this code Also, I think HEIC format should be added because when I take a picture with iPhone 12, the default format is HEIC

parasharrajat commented 2 months ago

In that case, can you please update your proposal to suggest changes for conversion as well. The proposal should be complete solution for allow the Heic images to be uploaded to the server and rendered on the UI.

nyomanjyotisa commented 2 months ago

Proposal updated @parasharrajat

melvin-bot[bot] commented 2 months ago

@Beamanator @johnmlee101 @kevinksullivan @parasharrajat this issue is now 4 weeks old, please consider:

Finding a contributor to fix the bug
Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

parasharrajat commented 2 months ago

@nyomanjyotisa's proposal looks good to me.

:ribbon: :eyes: :ribbon: C+ reviewed

melvin-bot[bot] commented 2 months ago

Current assignees @johnmlee101 and @Beamanator are eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

melvin-bot[bot] commented 2 months ago

📣 @nyomanjyotisa 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

parasharrajat commented 2 months ago

Oops, I just remembered that there was long discussion on the use heic2any. I think we need to bring this to slack again.

parasharrajat commented 2 months ago

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

@nyomanjyotisa Could you please expand on this? Why can't we use it on web?

Docs says web too...

nyomanjyotisa commented 1 month ago

@parasharrajat I've tried testing with expo-image-manipulator. It works well on native Android and iOS but does not work on the web version. I’ve attached some videos of the test below. Also, here is the draft branch using expo-image-manipulator. Could you please check it?

MacOS: Chrome *

https://github.com/user-attachments/assets/e7f0f46d-dcaf-47dd-848e-ba8d98f131e6

Android: Native

https://github.com/user-attachments/assets/5a6e7295-7b06-4f08-bbd5-5031819ad4a4

iOS: Native

https://github.com/user-attachments/assets/baafa3ac-87a0-4240-8332-593e16c0e308

kevinksullivan commented 1 month ago

Looping in another BZ member as I'm going OOO

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @RachCHopkins (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.

parasharrajat commented 1 month ago

Ongoing discussion https://expensify.slack.com/archives/C01GTK53T8Q/p1726086099438869.

RachCHopkins commented 1 month ago

@parasharrajat looks like Jack replied to you in Slack.

parasharrajat commented 1 month ago

@RachCHopkins Main discussion is happening on the sub-thread which jack posted.

parasharrajat commented 1 month ago

@RachCHopkins @kevinksullivan Could you please take part in https://expensify.slack.com/archives/C01GTK53T8Q/p1707130645906709 to decide the next steps for this issue and PR? Hopefully this will speed up the process here.

RachCHopkins commented 1 month ago

I don't think I'm really qualified to have an opinion there, other than as an iPhone user I want things to work nicely. @johnmlee101 @Beamanator what is the way forward here?

melvin-bot[bot] commented 1 month ago

@Beamanator, @johnmlee101, @parasharrajat, @RachCHopkins, @nyomanjyotisa Huh... This is 4 days overdue. Who can take care of this?

Beamanator commented 1 month ago

I just posted in slack (here) to offer 1 potential alternate solution before we go through the new lib process again, just because people already thought "this was an edge case" so it could be nice to do a small improvement instead of implement the full feature w/ this lib

melvin-bot[bot] commented 1 month ago

@Beamanator, @johnmlee101, @parasharrajat, @RachCHopkins, @nyomanjyotisa 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

melvin-bot[bot] commented 1 month ago

@Beamanator, @johnmlee101, @parasharrajat, @RachCHopkins, @nyomanjyotisa 10 days overdue. I'm getting more depressed than Marvin.

RachCHopkins commented 1 month ago

@Beamanator was there any resolution there?

Beamanator commented 1 month ago

Nobody commented 😅 :forever-alone: so I bumped the thread 😬 🙏

melvin-bot[bot] commented 1 month ago

@Beamanator, @johnmlee101, @parasharrajat, @RachCHopkins, @nyomanjyotisa 12 days overdue. Walking. Toward. The. Light...

melvin-bot[bot] commented 3 weeks ago

This issue has not been updated in over 14 days. @Beamanator, @johnmlee101, @parasharrajat, @RachCHopkins, @nyomanjyotisa eroding to Weekly issue.

Expensify / App