Add 2MB per-file size limit for Corpay document uploads#88426
Conversation
The UploadFile component had a totalFilesSizeLimit prop but no per-file size validation, allowing individual files larger than the backend's limit to be uploaded. This adds a maxFileSize prop to UploadFile and passes it (2MB) in both the Signer and BeneficialOwner upload pages. Co-authored-by: madmax330 <madmax330@users.noreply.github.com> Co-authored-by: Maxence Coulibaly <madmax330@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
The only failing check is checklist (PR Author Checklist), which is expected — all code checks (ESLint, typecheck, tests, prettier, builds, spellcheck, etc.) are passing. The checklist failure requires the human co-author to fill out the test steps, QA steps, and screenshots sections in the PR body. No code fix is needed. |
|
@jayeshmangwani Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
@MrMuzyk can you review this since you worked on the Non USD flow? |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6c62864a0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
The only failing check is |
|
@abzokhattab Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
| ALLOWED_FILE_TYPES: ['pdf', 'jpg', 'jpeg', 'png'], | ||
| FILE_LIMIT: 1, | ||
| MAX_FILE_SIZE: 2097152, | ||
| TOTAL_FILES_SIZE_LIMIT: 5242880, |
There was a problem hiding this comment.
The 'Explanation of change' in the PRs description already touches on that but this PR doesn't fix it - I think it could've but its outside of the scope of this issue.
CONST is defined but it's not referenced anywhere and despite the texts on the pages we do not enforce this total 5mb file size, anywhere, at all.
There was a problem hiding this comment.
Yeah I will fix this in another PR
|
@madmax330 Do you also want me to do review checklist here? |
Move ALLOWED_FILE_TYPES, FILE_LIMIT, MAX_FILE_SIZE, and TOTAL_FILES_SIZE_LIMIT to a new CORPAY_DOCUMENT const group since these consts are used in both USD and non-USD flows (e.g., DocusignFullStep is also used on the wallet page for global reimbursements). Co-authored-by: Maxence Coulibaly <madmax330@users.noreply.github.com>
That would be great! |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid-native.mp4Android: mWeb Chromeandroid.web.mp4iOS: HybridAppios-native.mp4iOS: mWeb Safariios-web.mp4MacOS: Chrome / Safariweb.mp4 |
|
@madmax330 Noticed one small thing. When we try to submit without any file we get an error 'Field is required'. If we then try to upload file that exceeds the limit we still get the same error. Not sure whether we want to block on this - other cases work fine minor-issue.mov |
Not sure whether we want to block on this - other cases work fine @MelvinBot can you address this? |
When a form has a 'Field is required' validation error and the user then picks an oversized file, the size error was hidden because form validation errors (errors[inputID]) take precedence over errorFields in FormProvider. Clear form validation errors before setting upload-specific error fields. Co-authored-by: Maxence Coulibaly <madmax330@users.noreply.github.com>
|
Fixed — the issue was that form validation errors ( |
|
Didnt work. Here is the diff that works index ffe41a3984e..cb461095c5b 100644
--- a/src/components/SubStepForms/DocusignFullStep.tsx
+++ b/src/components/SubStepForms/DocusignFullStep.tsx
@@ -1,8 +1,8 @@
-import React, {useCallback, useState} from 'react';
+import React, {useCallback, useRef, useState} from 'react';
import Button from '@components/Button';
import FormProvider from '@components/Form/FormProvider';
import InputWrapper from '@components/Form/InputWrapper';
-import type {FormInputErrors, FormOnyxKeys, FormOnyxValues} from '@components/Form/types';
+import type {FormInputErrors, FormOnyxKeys, FormOnyxValues, FormRef} from '@components/Form/types';
import InteractiveStepWrapper from '@components/InteractiveStepWrapper';
import Text from '@components/Text';
import UploadFile from '@components/UploadFile';
@@ -61,6 +61,8 @@ function DocusignFullStep<TFormID extends keyof OnyxFormValuesMapping>({
const styles = useThemeStyles();
const {environmentURL} = useEnvironment();
+ const formRef = useRef<FormRef | null>(null);
+
const [uploadedFiles, setUploadedFiles] = useState<FileObject[]>(defaultValue);
const validate = useCallback(
@@ -87,6 +89,7 @@ function DocusignFullStep<TFormID extends keyof OnyxFormValuesMapping>({
return;
}
+ formRef.current?.resetFormFieldError(inputID as string);
clearErrors(formID);
setErrorFields(formID, {[inputID]: {onUpload: error}});
};
@@ -107,6 +110,7 @@ function DocusignFullStep<TFormID extends keyof OnyxFormValuesMapping>({
startStepIndex={startStepIndex}
>
<FormProvider
+ ref={formRef}
formID={formID}
submitButtonText={translate('common.submit')}
onSubmit={onSubmit}
diff --git a/src/pages/ReimbursementAccount/NonUSD/BeneficialOwnerInfo/BeneficialOwnerDetailsFormSubSteps/Documents.tsx b/src/pages/ReimbursementAccount/NonUSD/BeneficialOwnerInfo/BeneficialOwnerDetailsFormSubSteps/Documents.tsx
index 0be181053d5..5f6395c3f56 100644
--- a/src/pages/ReimbursementAccount/NonUSD/BeneficialOwnerInfo/BeneficialOwnerDetailsFormSubSteps/Documents.tsx
+++ b/src/pages/ReimbursementAccount/NonUSD/BeneficialOwnerInfo/BeneficialOwnerDetailsFormSubSteps/Documents.tsx
@@ -1,8 +1,8 @@
-import React, {useCallback, useMemo, useState} from 'react';
+import React, {useCallback, useMemo, useRef, useState} from 'react';
import {View} from 'react-native';
import FormProvider from '@components/Form/FormProvider';
import InputWrapper from '@components/Form/InputWrapper';
-import type {FormInputErrors, FormOnyxValues} from '@components/Form/types';
+import type {FormInputErrors, FormOnyxValues, FormRef} from '@components/Form/types';
import Text from '@components/Text';
import UploadFile from '@components/UploadFile';
import useLocalize from '@hooks/useLocalize';
@@ -45,6 +45,8 @@ function Documents({onNext, isEditing, ownerBeingModifiedID}: DocumentsProps) {
[codiceFiscaleInputID]: Array.isArray(reimbursementAccountDraft?.[codiceFiscaleInputID]) ? (reimbursementAccountDraft?.[codiceFiscaleInputID] ?? []) : [],
};
+ const formRef = useRef<FormRef | null>(null);
+
const [uploadedProofOfOwnership, setUploadedProofOfOwnership] = useState<FileObject[]>(defaultValues[proofOfOwnershipInputID]);
const [uploadedCopyOfID, setUploadedCopyOfID] = useState<FileObject[]>(defaultValues[copyOfIDInputID]);
const [uploadedAddressProof, setUploadedAddressProof] = useState<FileObject[]>(defaultValues[addressProofInputID]);
@@ -78,6 +80,7 @@ function Documents({onNext, isEditing, ownerBeingModifiedID}: DocumentsProps) {
return;
}
+ formRef.current?.resetFormFieldError(inputID);
clearErrors(ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM);
setErrorFields(ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM, {[inputID]: {onUpload: error}});
};
@@ -98,6 +101,7 @@ function Documents({onNext, isEditing, ownerBeingModifiedID}: DocumentsProps) {
return (
<FormProvider
+ ref={formRef}
formID={ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM}
submitButtonText={translate(isEditing ? 'common.confirm' : 'common.next')}
validate={validate}
diff --git a/src/pages/ReimbursementAccount/NonUSD/SignerInfo/subSteps/UploadDocuments.tsx b/src/pages/ReimbursementAccount/NonUSD/SignerInfo/subSteps/UploadDocuments.tsx
index f413e08a6df..b36fc2eab87 100644
--- a/src/pages/ReimbursementAccount/NonUSD/SignerInfo/subSteps/UploadDocuments.tsx
+++ b/src/pages/ReimbursementAccount/NonUSD/SignerInfo/subSteps/UploadDocuments.tsx
@@ -1,10 +1,10 @@
-import React, {useEffect, useState} from 'react';
+import React, {useEffect, useRef, useState} from 'react';
import {View} from 'react-native';
import Button from '@components/Button';
import DotIndicatorMessage from '@components/DotIndicatorMessage';
import FormProvider from '@components/Form/FormProvider';
import InputWrapper from '@components/Form/InputWrapper';
-import type {FormInputErrors, FormOnyxKeys, FormOnyxValues} from '@components/Form/types';
+import type {FormInputErrors, FormOnyxKeys, FormOnyxValues, FormRef} from '@components/Form/types';
import Text from '@components/Text';
import UploadFile from '@components/UploadFile';
import useLocalize from '@hooks/useLocalize';
@@ -50,6 +50,8 @@ function UploadDocuments({onNext, isEditing}: UploadDocumentsProps) {
[CODICE_FISCALE]: Array.isArray(reimbursementAccountDraft?.[CODICE_FISCALE]) ? (reimbursementAccountDraft?.[CODICE_FISCALE] ?? []) : [],
};
+ const formRef = useRef<FormRef | null>(null);
+
const [uploadedIDs, setUploadedID] = useState<FileObject[]>(defaultValues[COPY_OF_ID]);
const [uploadedProofsOfAddress, setUploadedProofOfAddress] = useState<FileObject[]>(defaultValues[ADDRESS_PROOF]);
const [uploadedProofsOfDirectors, setUploadedProofsOfDirectors] = useState<FileObject[]>(defaultValues[PROOF_OF_DIRECTORS]);
@@ -97,6 +99,7 @@ function UploadDocuments({onNext, isEditing}: UploadDocumentsProps) {
return;
}
+ formRef.current?.resetFormFieldError(inputID);
clearErrors(ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM);
setErrorFields(ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM, {[inputID]: {onUpload: error}});
};
@@ -109,6 +112,7 @@ function UploadDocuments({onNext, isEditing}: UploadDocumentsProps) {
return (
<FormProvider
+ ref={formRef}
formID={ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM}
submitButtonText={translate(isEditing ? 'common.confirm' : 'common.next')}
onSubmit={handleSubmitWithDownload}``` |
|
@MelvinBot can you undo your fix and apply the diff mentioned above? #88426 (comment) |
…wing upload errors Replace the clearErrors-only approach with formRef.resetFormFieldError to properly clear per-field validation errors (e.g. 'Field is required') before setting upload-specific errorFields. This ensures the upload size error is displayed when a form validation error was already present. Diff provided by MrMuzyk. Co-authored-by: Maxence Coulibaly <madmax330@users.noreply.github.com>
|
Applied |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product
|
Tests well, approved |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @MariaHCD has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/MariaHCD in version: 9.3.69-0 🚀
Bundle Size Analysis (Sentry): |
|
Yes — help site changes are required. The PR adds a 2 MB per-file size limit for Corpay document uploads, but none of the global reimbursement help articles mention this constraint. I've created a draft PR with the updates: #90015 Changes made to 5 articles:
|
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.3.69-18 🚀
|
Update all global reimbursement help site articles to document the new 2 MB per-file size limit for Corpay document uploads introduced in Expensify#88426. Co-authored-by: Maxence Coulibaly <madmax330@users.noreply.github.com>
Explanation of Change
The
UploadFilecomponent validates total cumulative file size (totalFilesSizeLimit) but has no per-file size validation. The Corpay onboarding document upload pages (Signer and BeneficialOwner) don't passtotalFilesSizeLimiteither, so there's no frontend size enforcement at all. This allows files larger than the backend's limit to be uploaded to S3, which then fail when theUploadCorpayOnboardingDocumentsbackend job tries to forward them to Corpay.This PR:
MAX_FILE_SIZEconstant of 2MB (2,097,152 bytes) toCONST.NON_USD_BANK_ACCOUNTmaxFileSizeprop to theUploadFilecomponent that validates each individual file's size before uploadmaxFileSizeto allUploadFileinstances in both the Signer and BeneficialOwner document upload pagesFixed Issues
$ https://github.com/Expensify/Expensify/issues/568619
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validates your changed worked.
Verify that no errors appear in the JS console
Create a new account in ND
Create a non usd workspace
Go to the workspace and wokflows and add a bank account and go through the flow until you get to a file upload. Then try to add a file > 2MB and you should see the following error:
Offline tests
N/A - file size validation is a client-side check that does not depend on network state.
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari