[$500] Android - IOU - Validation message not shown when file above 25mb for Smart Recipe was selected

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Issue found when executing PR #27089

Action Performed:

1. Open app
2. Tap FAB
3. Tap on Request Money
4. Select Smart Recipe tab
5. Tap on add file from gallery button
6. Select any photo larger than 25mb
7. Select any attendee
8. Tap on Request button

Expected Result:

The validation message should be show when user tries use the photo larger than 25mb for Smart Recipe

Actual Result:

Validation message not shown and user able to request money with smart recipe option when file is larger than 25mb

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.69-0

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6199398_screen-20230913-181013.mp4

Bug6199398_screen-20230913-195040.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01898f68a06cf0cea0
• Upwork Job ID: 1703675063206494208
• Last Price Increase: 2023-09-18
• Automatic offers:
• ishpaul777 | Contributor | 26791640

[melvin-bot]

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

[melvin-bot]

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

[ishpaul777]

Proposal

Problem

Validation message not shown when file above 25mb for Smart Recipe was selected

Root cause

There is no validation of file size or any other validation on after image is picked

https://github.com/ishpaul777/App/blob/1bdc5acc12f982b4f6e37bfadf9060ac3d2d3b3c/src/pages/iou/ReceiptSelector/index.native.js#L262

Changes

Validate the image picked by user for fileSize.(Asset picked by user has a fileSize property https://www.npmjs.com/package/react-native-image-picker)

                    showImagePicker(launchImageLibrary)
                            .then((receiptImage) => {
                                if (receiptImage[0].fileSize > CONST.IOU.MAX_RECEIPT_FILE_SIZE) {
                                    // show error modal if the file size is too large
                                }  else if (receiptImage[0].fileSize < CONST.IOU.MIN_RECEIPT_FILE_SIZE) {
                                    // show error modal if the file size is too small
                                }

[hoangzinh]

Proposal

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

Android - IOU - Validation message not shown when file above 25mb for Smart Recipe was selected

What is the root cause of that problem?

In the callback after select file in Native platforms, we haven't validate the valid of file, including file type & file size yet

App/src/pages/iou/ReceiptSelector/index.native.js

Lines 260 to 269 in 6764fd1

	onPress={() => {
	showImagePicker(launchImageLibrary)
	.then((receiptImage) => {
	IOU.setMoneyRequestReceipt(receiptImage[0].uri, receiptImage[0].fileName);
	IOU.navigateToNextPage(props.iou, iouType, reportID, props.report);
	})
	.catch(() => {
	Log.info('User did not select an image from gallery');
	});
	}}

Like we did for default platform here

App/src/pages/iou/ReceiptSelector/index.js

Lines 80 to 82 in 6764fd1

	if (!ReceiptUtils.validateReceipt(file)) {
	return;
	}

App/src/libs/ReceiptUtils.js

Lines 12 to 30 in 6764fd1

	function validateReceipt(file) {
	const {fileExtension} = FileUtils.splitExtensionFromFileName(lodashGet(file, 'name', ''));
	if (_.contains(CONST.API_ATTACHMENT_VALIDATIONS.UNALLOWED_EXTENSIONS, fileExtension.toLowerCase())) {
	Receipt.setUploadReceiptError(true, 'attachmentPicker.wrongFileType', 'attachmentPicker.notAllowedExtension');
	return false;
	}
	if (lodashGet(file, 'size', 0) > CONST.API_ATTACHMENT_VALIDATIONS.MAX_SIZE) {
	Receipt.setUploadReceiptError(true, 'attachmentPicker.attachmentTooLarge', 'attachmentPicker.sizeExceeded');
	return false;
	}
	if (lodashGet(file, 'size', 0) < CONST.API_ATTACHMENT_VALIDATIONS.MIN_SIZE) {
	Receipt.setUploadReceiptError(true, 'attachmentPicker.attachmentTooSmall', 'attachmentPicker.sizeNotMet');
	return false;
	}
	return true;
	}

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

1. In the callback of Native platforms here, we need to reuse above util ReceiptUtils.validateReceipt to validate same as we did for default platform.
2. In native platform, the prop to get size is file.fileSize, instead of file.size, so in the util ReceiptUtils.validateReceipt, we need to modify a bit to get file size in either file.size or file.fileSize
3. In order to show attachment error popup, we need to render the ConfirmModal same as we did for default platform too

   App/src/pages/iou/ReceiptSelector/index.js

   Lines 143 to 151 in 6764fd1

   	<ConfirmModal
   	title={attachmentInvalidReasonTitle ? translate(attachmentInvalidReasonTitle) : ''}
   	onConfirm={Receipt.clearUploadReceiptError}
   	onCancel={Receipt.clearUploadReceiptError}
   	isVisible={isAttachmentInvalid}
   	prompt={attachmentInvalidReason ? translate(attachmentInvalidReason) : ''}
   	confirmText={translate('common.close')}
   	shouldShowCancelButton={false}
   	/>

Result

[sakluger]

Looks like this may be a regression from #25865, I've commented in that issue for clarification.

[sakluger]

Okay it's not a regression because we didn't ever have the size validation on mobile. As part of this fix, let's make sure we add the validation across all platforms where it hasnt been added yet.

[melvin-bot]

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

[melvin-bot]

Current assignee @sakluger is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[allroundexperts]

Thanks for your proposal @hoangzinh. However, I think your proposal has the same approach as previously mentioned by @ishpaul777.

As such, I think we should go with the first proposal.

@ishpaul777's proposal looks good to me. @ishpaul777 Let's use the validation function we already have instead of re-writing the validation logic.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @deetergp, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[deetergp]

Good enough for me, let's go with @ishpaul777's proposal.

[melvin-bot]

📣 @allroundexperts Please request via NewDot manual requests for the Reviewer role ($500)

[melvin-bot]

📣 @ishpaul777 🎉 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]

This issue has not been updated in over 15 days. @deetergp, @sakluger, @allroundexperts, @ishpaul777 eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[deetergp]

@ishpaul777 Any updates? Do we need to raise the price and make this generally available agin?

[ishpaul777]

Sorry for no update @deetergp, i don't think raising price is necessary but i'd request for reset of days limit, i already completed the most part but there is a unresolved issue that requires you thoughts and approval
#27960 (comment)

[ishpaul777]

gentle bump @deetergp

[deetergp]

My apologies but the thread on this one is very hard to follow. Can someone please summarize what is going on and where we are at with this issue? @ishpaul777 @allroundexperts

[situchan]

I suggest to put this on hold for #29963

[ishpaul777]

On IOS, sometimes the transition when opening the modal is not smooth, I believe the Root cause is that closing of photos gallery and Confirm modal overlaps, so i have added a delay in opening in the modal @allroundexperts has not approved the change and didn't provide any feedback

[allroundexperts]

On IOS, sometimes the transition when opening the modal is not smooth, I believe the Root cause is that closing of photos gallery and Confirm modal overlaps, so i have added a delay in opening in the modal @allroundexperts has not approved the change and didn't provide any feedback

@ishpaul777 Please see #27960 (comment)

[ishpaul777]

Okay i see there is some missunderstanding i replied everytime @allroundexperts bumped but my reply didn't get to them because of some issue with GH thread showing pending (i didn't realized the pending label is for not delivered)

[ishpaul777]

@situchan Is #29963 solving this issue too?

[situchan]

@situchan Is #29963 solving this issue too?

I think so

[ishpaul777]

I'd like to understand why we are using the Native Alert Modal instead of a Custom Modal in our project. This might create inconsistency since we use Custom Modals on the web. Is there a specific reason for this choice?

---

I'm concerned that my work in #27960 might be wasted if the issue is already addressed in #29963. Should I close the PR?

[situchan]

I'd like to understand why we are using the Native Alert Modal instead of a Custom Modal in our project. This might create inconsistency since we use Custom Modals on the web. Is there a specific reason for this choice?

Native alert is already used in various parts: corrupted image, download, permission, camera errors

[ishpaul777]

I meant for similar case like validation error on web has a custom modal I think?

[situchan]

I meant for similar case like validation error on web has a custom modal I think?

yes because web native alert is ugly

[situchan]

If we should use custom alert for all platforms for consistency, we should fix it for all cases, not only this one.
Btw, it's new issue and should get design approval. Out of scope for this one.

[ishpaul777]

In summary, we are addressing this issue in #29663. I agree with @situchan we place this issue on hold for #29663. Additionally, I'm closing #27960 since there's already a pull request handling the issue.

Given that there is decent effort invested, should I expect of receiving a partial payment for this issue? Do we agree 👍 or 👎

[allroundexperts]

Hm... Yes, holding makes sense. @ishpaul777 I'm not sure if we consider partial payments for issues that are on hold.

[melvin-bot]

@deetergp, @sakluger, @allroundexperts, @ishpaul777, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.