[HOLD for payment 2025-02-04] [$250] PR - No error message displayed for greater than 15 mb jpg file

[mitarachim]

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: v9.0.80-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: #53902
Email or phone of affected tester (no customers): applausetester+en@applause.expensifail.com
Issue reported by: Applause Internal Team
Device used: Macbook Pro 2023 / Sonoma 14.4.1
App Component: Other

Action Performed:

1. Open the New Expensify Desktop app
2. Click on FAB > Submit expense
3. Enter an amount, click on 'Next'
4. Choose any recipient
5. Click on three dots at top right corner in expense creation screen
6. Click 'Add receipt'
7. Select a receipt equal or greater than 10mb.

Expected Result:

Verify that 'Attachment is too large - Attachment size is larger than 10 MB limit' is displayed

Actual Result:

Able to upload the file with no issues

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6704689_1735685408972.PR_53902_Desktop.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021874862385024395150
• Upwork Job ID: 1874862385024395150
• Last Price Increase: 2025-01-16
• Automatic offers:
• nkdengineer | Contributor | 105776417

Issue Owner

Current Issue Owner: @MitchExpensify

[melvin-bot]

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

[Themoonalsofall]

@mitarachim @MitchExpensify Sorry for not clarifying the steps in the previous PR. Here, we only display "Attachment is too large - Attachment size is larger than the 10 MB limit" for receipts that are not images.

[mitarachim]

@Themoonalsofall Okay thank you for clarify, so should be truly receipt in .jpg or .png ?

[Themoonalsofall]

@mitarachim From what I see in the source code, for receipts in image formats like .jpg or .png, we don’t limit the file size. The size restriction only applies to formats like PDF, ZIP, and others.

[mitarachim]

@Themoonalsofall already ask tester to re-test.
seems tester not available at the moment , i also try in my side and can see the restriction while using PDF larger than 10 MB

Screen.Recording.2025-01-01.at.3.02.08.PM.mp4

[melvin-bot]

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

[melvin-bot]

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

[nkdengineer]

Proposal

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

What is the root cause of that problem?

For the image, we will resize it if the file is larger than 24MB based on the expected here. So it's expected that no error message displays for the greater than 15MB image. But we have a problem here: the max size image is 1024 * 1024 while the max dimension is 2400 that can causes the wrong resolution image when we upload the large image

App/src/libs/fileDownload/FileUtils.ts

Line 320 in fdd1db8

const scaleFactor = CONST.MAX_IMAGE_DIMENSION / (width < height ? height : width);

App/src/libs/fileDownload/FileUtils.ts

Lines 328 to 331 in fdd1db8

	if (!file || !Str.isImage(file.name ?? '') || (file?.size ?? 0) <= CONST.API_ATTACHMENT_VALIDATIONS.RECEIPT_MAX_SIZE) {
	return Promise.resolve(file);
	}
	return getImageDimensionsAfterResize(file).then(({width, height}) => getImageManipulator({fileUri: file.uri ?? '', width, height, fileName: file.name ?? '', type: file.type}));

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

We should change CONST.API_ATTACHMENT_VALIDATIONS.RECEIPT_MAX_SIZE) to CONST.API_ATTACHMENT_VALIDATIONS.MAX_SIZE to match with the previous expected here

App/src/libs/fileDownload/FileUtils.ts

Lines 328 to 331 in fdd1db8

	if (!file || !Str.isImage(file.name ?? '') || (file?.size ?? 0) <= CONST.API_ATTACHMENT_VALIDATIONS.RECEIPT_MAX_SIZE) {
	return Promise.resolve(file);
	}
	return getImageDimensionsAfterResize(file).then(({width, height}) => getImageManipulator({fileUri: file.uri ?? '', width, height, fileName: file.name ?? '', type: file.type}));

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

Or we can change the CONST.MAX_IMAGE_DIMENSION to 1024 and then resize the image correctly with a max size of 10MB.

App/src/libs/fileDownload/FileUtils.ts

Line 320 in fdd1db8

const scaleFactor = CONST.MAX_IMAGE_DIMENSION / (width < height ? height : width);

[jayeshmangwani]

It seems this PR (#53902) has been reverted. @dominictb and @madmax330, will this issue be addressed in the new PR, or should we continue evaluating proposals on this issue?

[dominictb]

@mitarachim Please share the attachment used in OP here so we can retest it. For any attachment-related issues, we should always share the attachment as well. Thanks.

[mitarachim]

@dominictb 1 file .jpg from tester that i have compress in .zip , please extract if needed that file .
1 file .pdf from me while re-test here
20mb.pdf
Bug6704689_1735685602908!15mb.jpg.zip

[dominictb]

@mitarachim Thanks for the responsiveness! I will retest with these files and reply here soon whether to continue this issue.

[melvin-bot]

@jayeshmangwani, @MitchExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[dominictb]

Okay I finished retesting with the changes in #54728.

1. For the 20 MB image, it would be automatically reduced in size so we won't show the error >> No error is expected
2. For the PDF, at the time of QA retesting [HOLD for payment 2025-02-04] [$250] PR - No error message displayed for greater than 15 mb jpg file #54700 (comment), the PR was reverted so we wouldn't have seen the error. But with the changes in the new PR Fix unable to upload a receipt size larger than 15mb #54728, it would obviously show error

Thus I think we can close this issue then cc @MitchExpensify @jayeshmangwani.

I'll ask the author to update test steps to exclude image type due to point (1).

[MitchExpensify]

Do you agree this can be closed @jayeshmangwani ?

[jayeshmangwani]

Yes @MitchExpensify, we can close this issue for this PR #54728

[nkdengineer]

@MitchExpensify Based on #45448. We should only automatically resize the image > 24MB (Now it's automatically resizing the image > 10MB although CONST.MAX_IMAGE_DIMENSION is 2400). So we should fix to match with the previous expected here.

[melvin-bot]

📣 @nkdengineer 🎉 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 📖

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• fix: max image file size #55499

If no regressions arise, payment will be issued on 2025-02-04. 🎊

For reference, here are some details about the assignees on this issue:

• @jayeshmangwani requires payment through NewDot Manual Requests
• @nkdengineer requires payment automatic offer (Contributor)

[melvin-bot]

@jayeshmangwani @MitchExpensify @jayeshmangwani The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[MitchExpensify]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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:

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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:

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal Template

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Precondition:

Test:

Do we agree 👍 or 👎

[MitchExpensify]

Payment summary:

• $250 @jayeshmangwani requires payment through NewDot Manual Requests
• $250 @nkdengineer requires payment automatic offer (Contributor)

Reminder on the BZ steps above, @jayeshmangwani - Thank you!

[jayeshmangwani]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other: Wrong QA step led to this bug. We have different expected results for the image and PDF file of the receipt

Where bug was reported:

• 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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: The issue initially arose because we had the wrong QA steps for this PR: Fix unable to upload a receipt size larger than 15mb #53902
  Comment from the contributor who raised the above PR: [HOLD for payment 2025-02-04] [$250] PR - No error message displayed for greater than 15 mb jpg file #54700 (comment)

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal

1. Open the app and click the FAB button.
2. Create an expense and select the Scan option.
3. Choose an image receipt with a size of 24MB.
4. Verify that:
   • The app takes some time to resize the image.
   • The file uploads successfully without any issues.

Do we agree 👍 or 👎

[luacmartins]

I don't think we need a specific one for this issue. We already test receipt uploads, which should be enough.

[melvin-bot]

@luacmartins, @jayeshmangwani, @MitchExpensify, @nkdengineer Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[jayeshmangwani]

@MitchExpensify, issue is ready to be paid.

[melvin-bot]

@luacmartins, @jayeshmangwani, @MitchExpensify, @nkdengineer 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[luacmartins]

Bump for payment @MitchExpensify

[MitchExpensify]

Paid @nkdengineer, @jayeshmangwani needs payment in New Expensify as a next step - Closing

[garrettmknight]

$250 approved for @jayeshmangwani