Don't allow upload of large attachments [hold for payment on August 5th]

[rushatgabhane]

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

---

I'm assuming API allows max attachment size of 100 MB. Please correct me if I'm wrong.

Action Performed:

1. Login to E.cash
2. Click add attachment.
3. Select a file larger than 100 MB, and send it.

Expected Result:

Should display a message like "Attachment is larger than 100 MB"
and not allow upload of the attachment.

Actual Result:

The attachment is allowed to upload even though the API doesn't allow uploads larger than 100 MB.
And a forever spinner gets displayed.

Workaround:

N.A.

Platform:

Where is this issue occurring?

Web ✔️
iOS ✔️
Android ✔️
Desktop App ✔️
Mobile Web ✔️

Version Number:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:

View all open jobs on Upwork

[MelvinBot]

Triggered auto assignment to @bfitzexpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[rushatgabhane]

Proposal

In components/AttachmentModal

For attachments larger than 100 MB,
show the message "Attachment is larger than 100 MB limit", and disable the send button.

English and Spanish message will be added, and size limit will be set in CONST.js

[MelvinBot]

Triggered auto assignment to @yuwenmemon (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[yuwenmemon]

I'm curious how we picture this error message to show? I don't think we should even render the preview and disable the upload button here. Since a large file would probably be cumbersome to render anyway:

We should probably just flash a separate, solo error message

[yuwenmemon]

Code where this happens:

App/src/components/AttachmentPicker/index.js

Lines 19 to 28 in beb40ee

	onChange={(e) => {
	const file = e.target.files[0];
	if (file) {
	file.uri = URL.createObjectURL(file);
	this.onPicked(file);
	}
	// Cleanup after selecting a file to start from a fresh state
	this.fileInput.value = null;

[yuwenmemon]

Spanish translation: El documento adjunto supera el límite de 100 MB

[quinthar]

Yes, if the image is too big, I don't think we'd show a preview (especially because odds are it's a format that won't have a preview).

…

On Mon, Jul 26, 2021 at 2:28 PM Yuwen Memon ***@***.***> wrote: I'm curious how we picture this error message to show? I don't think we should even render the preview and disable the upload button here. Since a large file would probably be cumbersome to render anyway: [image: Screen Shot 2021-07-26 at 2 27 35 PM] <https://user-images.githubusercontent.com/4741899/127061563-dd141818-e7c9-44b7-a4f7-aa87ae1e403a.png> We should probably just flash a separate, solo error message — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3926 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEMNUTPXAUYSAQ456TA56DTZXHRLANCNFSM5AAFC6YQ> .

[rushatgabhane]

Yeah, gonna conditionally render text vs preview.
This is similiar to how whatsapp web handles it.

[MelvinBot]

Triggered auto assignment to @muttmuure (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[MelvinBot]

Triggered auto assignment to @kadiealexander (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[yuwenmemon]

Hmmm... @rushatgabhane what if you use the ConfirmModal instead? What would that look like?
https://github.com/Expensify/App/blob/main/src/components/ConfirmModal.js

[rushatgabhane]

@yuwenmemon yeah, it looks better! And we don't have to deal with the download attachment button.

I'll fine tune the title and promt text in the PR.

[quinthar]

Awesome! What is our actual limit? I'm assuming a lot smaller than that.

…

On Mon, Jul 26, 2021 at 5:07 PM Rushat Gabhane ***@***.***> wrote: @yuwenmemon <https://github.com/yuwenmemon> yeah, it looks better! [image: image] <https://user-images.githubusercontent.com/29673073/127074603-57682a19-4ca6-4c2a-b620-e320816a380c.png> — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3926 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEMNURVFMTBCJYBM4XZ5ILTZX2FXANCNFSM5AAFC6YQ> .

[yuwenmemon]

@rushatgabhane quick edit on the Spanish text, it's actually: El archivo adjunto supera el límite de 100 MB

[yuwenmemon]

What is our actual limit? I'm assuming a lot smaller than that.

Yeah, @rushatgabhane based on Expensify's internal ReceiptAPI the limit is actually 50MB:

/**
 * Class ReceiptAPI.
 *
 * Wrapper for API receipt calls
 */
class ReceiptAPI
{
    use LogTrait;

    // 50 megabytes in bytes
    const SIZE_LIMIT = 52428800;

[kadiealexander]

Wow, this moved quickly! Upwork post here: https://www.upwork.com/jobs/~013cc27ddfb15c57a3

[MelvinBot]

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

[yuwenmemon]

I believe @rushatgabhane is working on this? (And it was already merged?)

[kadiealexander]

Yeah, it moved really quickly. Was just getting the Upwork post up to arrange payment for @rushatgabhane :)

[rushatgabhane]

I believe @rushatgabhane is working on this? (And it was already merged?)

Yes :)

[kadiealexander]

Issue #4356 must be resolved.

[rushatgabhane]

@kadiealexander resolved and closed!

[kadiealexander]

Thanks @rushatgabhane! The original PR was merged 5 days ago, so will process payment in 2 more days. :)

[kadiealexander]

Paid with a bonus for finding the issue! Thanks @rushatgabhane 😁