-
Notifications
You must be signed in to change notification settings - Fork 3.6k
refactor: Improve and simplify attachment and file validation #70740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
chrispader
wants to merge
50
commits into
Expensify:main
Choose a base branch
from
margelo:@chrispader/attachment-validation-refactor
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+629
−437
Open
Changes from 8 commits
Commits
Show all changes
50 commits
Select commit
Hold shift + click to select a range
a058146
feat: improve attachment validation
chrispader 6739696
remove;: unused import
chrispader 542334d
Merge branch 'main' into pr/70740
chrispader 0c4a513
refactor: rename CONST variable block
chrispader 76ee521
fix: add missing CONST values
chrispader 0079017
fix: legacy CONST key
chrispader ff9c18c
fix: incorrect usage of `useFilesValidation` hook
chrispader 6035f9a
fix: update usage of validation functions
chrispader 33e2bfc
Merge branch 'main' into pr/70740
chrispader 3be89c0
fix: `getFileValidationErrorText` type error
chrispader 4f9692c
fix: use `Log` instaed of `console`
chrispader 662e189
fix: pass down validation options to validator function
chrispader 34063ca
fix: pass file objects instead of wrapper struct
chrispader f014e7f
refactor: remove unused `getConfirmModalPrompt` function
chrispader 33cd2be
refactor: refactor `useFilesValidation` hook for more readability
chrispader 843acd4
test: fix test cases
chrispader 02e9868
test: fix file error cases with multiple files
chrispader 24019c5
test: add more test cases for attachment file validation
chrispader 5e585aa
fix: failing tests
chrispader fade3fd
fix: invalid options param
chrispader 662308c
refactor: convert to async functions
chrispader 9b5a71c
fix: remove unnecessary array vs. single instance checks
chrispader b6993e7
fix: replace `console.error`
chrispader 67425eb
Merge branch 'main' into pr/70740
chrispader b341077
fix: prettier
chrispader d07538e
Merge branch 'main' into pr/70740
chrispader 89479bb
refactor: AttachmentValidation functions
chrispader 3f73a1e
refactor: further simplify validation logic
chrispader 667393f
fix: reset on valid validation
chrispader 2d12c2e
Merge branch 'main' into pr/70740
chrispader 77ea218
fix: prettier
chrispader 27e0b96
Merge branch 'main' into pr/70740
chrispader 431f5be
fix: undo invalid change
chrispader 68bbd95
Merge branch 'main' into pr/70740
chrispader c192b3d
Merge branch 'main' into pr/70740
chrispader 696c820
fix: validate corrupted files
chrispader 8410652
refactor: merge validation error constants
chrispader f8fdeea
refactor: improve `useFilesValidation` code and make use of async-await
chrispader a8ae217
fix: only set validate multiple files state if more than 1
chrispader 5811936
refactor: simplify attachment validation logic
chrispader 893f03a
refactor: extract back `validateAttachmentFile` function
chrispader 2cab9ce
chore: update tests
chrispader 60a8d05
chore: update tests
chrispader 592e71b
fix: attachment validation in `ReportAddAttachmentModalContent`
chrispader 92e4dd5
test: update tests to match new `validateAttachmentFile` implementation
chrispader 3729607
refactor: rename reset callback
chrispader acee6c8
Merge branch 'main' into pr/70740
chrispader 73b5ca3
refactor: file validation logic
chrispader a1fb241
test: fix failing test
chrispader ae0deea
fix: always resolve file conversion promise
chrispader File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,206 @@ | ||
| import {Str} from 'expensify-common'; | ||
| import type {ValueOf} from 'type-fest'; | ||
| import CONST from '@src/CONST'; | ||
| import type {FileObject} from '@src/types/utils/Attachment'; | ||
| import {cleanFileName, hasHeicOrHeifExtension, isValidReceiptExtension, normalizeFileObject, validateImageForCorruption} from './fileDownload/FileUtils'; | ||
|
|
||
| type ValidatedFile = { | ||
| fileType: 'file' | 'uri'; | ||
| source: string; | ||
| file: FileObject; | ||
| }; | ||
|
|
||
| type SingleAttachmentValidResult = { | ||
| isValid: true; | ||
| validatedFile: ValidatedFile; | ||
| }; | ||
|
|
||
| type SingleAttachmentValidationError = ValueOf<typeof CONST.FILE_VALIDATION_ERRORS.SINGLE_FILE>; | ||
| type SingleAttachmentInvalidResult = { | ||
| isValid: false; | ||
| error: SingleAttachmentValidationError; | ||
| file: FileObject; | ||
| }; | ||
|
|
||
| type SingleAttachmentValidationResult = SingleAttachmentValidResult | SingleAttachmentInvalidResult; | ||
|
|
||
| function isSingleAttachmentValidationResult(result: unknown): result is SingleAttachmentValidationResult { | ||
| return typeof result === 'object' && result !== null && 'isValid' in result && typeof result.isValid === 'boolean' && ('validatedFile' in result || 'error' in result); | ||
| } | ||
|
|
||
| function validateAttachmentFile(file: FileObject, item?: DataTransferItem, isValidatingReceipts = false): Promise<SingleAttachmentValidationResult> { | ||
| if (!file) { | ||
| return Promise.resolve({isValid: false, error: CONST.FILE_VALIDATION_ERRORS.SINGLE_FILE.NO_FILE_PROVIDED, file}); | ||
| } | ||
|
|
||
| const maxFileSize = isValidatingReceipts ? CONST.API_ATTACHMENT_VALIDATIONS.RECEIPT_MAX_SIZE : CONST.API_ATTACHMENT_VALIDATIONS.MAX_SIZE; | ||
|
|
||
| const isImage = Str.isImage(file.name ?? ''); | ||
|
|
||
| if (isImage && hasHeicOrHeifExtension(file)) { | ||
| return Promise.resolve({isValid: false, error: CONST.FILE_VALIDATION_ERRORS.SINGLE_FILE.HEIC_OR_HEIF_IMAGE, file}); | ||
| } | ||
|
|
||
| if (!isImage && !hasHeicOrHeifExtension(file) && (file?.size ?? 0) > maxFileSize) { | ||
| return Promise.resolve({isValid: false, error: CONST.FILE_VALIDATION_ERRORS.SINGLE_FILE.FILE_TOO_LARGE, file}); | ||
| } | ||
|
|
||
| if (isValidatingReceipts && (file?.size ?? 0) < CONST.API_ATTACHMENT_VALIDATIONS.MIN_SIZE) { | ||
| return Promise.resolve({isValid: false, error: CONST.FILE_VALIDATION_ERRORS.SINGLE_FILE.FILE_TOO_SMALL, file}); | ||
| } | ||
|
|
||
| if (isValidatingReceipts && !isValidReceiptExtension(file)) { | ||
| return Promise.resolve({isValid: false, error: CONST.FILE_VALIDATION_ERRORS.SINGLE_FILE.WRONG_FILE_TYPE, file}); | ||
| } | ||
|
|
||
| let fileObject = file; | ||
| const fileConverted = file.getAsFile?.(); | ||
| if (fileConverted) { | ||
| fileObject = fileConverted; | ||
| } | ||
|
|
||
| if (!fileObject) { | ||
| return Promise.resolve({isValid: false, error: CONST.FILE_VALIDATION_ERRORS.SINGLE_FILE.FILE_INVALID, file}); | ||
| } | ||
|
|
||
| if (item && item.kind === 'file' && 'webkitGetAsEntry' in item) { | ||
| const entry = item.webkitGetAsEntry(); | ||
|
|
||
| if (entry?.isDirectory) { | ||
| return Promise.resolve({isValid: false, error: CONST.FILE_VALIDATION_ERRORS.SINGLE_FILE.FOLDER_NOT_ALLOWED, file}); | ||
| } | ||
| } | ||
|
|
||
| return isFileCorrupted(fileObject).then((corruptionResult) => { | ||
| if (!corruptionResult.isValid) { | ||
| return corruptionResult; | ||
| } | ||
|
|
||
| let validatedFile: ValidatedFile; | ||
|
|
||
| if (fileObject instanceof File) { | ||
| /** | ||
| * Cleaning file name, done here so that it covers all cases: | ||
| * upload, drag and drop, copy-paste | ||
| */ | ||
| let updatedFile = fileObject; | ||
| const cleanName = cleanFileName(updatedFile.name); | ||
| if (updatedFile.name !== cleanName) { | ||
| updatedFile = new File([updatedFile], cleanName, {type: updatedFile.type}); | ||
| } | ||
| const inputSource = URL.createObjectURL(updatedFile); | ||
| updatedFile.uri = inputSource; | ||
|
|
||
| validatedFile = { | ||
| fileType: 'file', | ||
| source: inputSource, | ||
| file: updatedFile, | ||
| }; | ||
|
|
||
| return {isValid: true, validatedFile}; | ||
| } | ||
|
|
||
| validatedFile = { | ||
| fileType: 'uri', | ||
| source: fileObject.uri ?? '', | ||
| file: fileObject, | ||
| }; | ||
|
|
||
| return {isValid: true, validatedFile}; | ||
| }); | ||
| } | ||
|
|
||
| type MultipleAttachmentsValidResult = { | ||
| isValid: true; | ||
| validatedFiles: ValidatedFile[]; | ||
| }; | ||
|
|
||
| type MultipleAttachmentsValidationError = ValueOf<typeof CONST.FILE_VALIDATION_ERRORS.MULTIPLE_FILES>; | ||
| type MultipleAttachmentsInvalidResult = { | ||
| isValid: false; | ||
| error: MultipleAttachmentsValidationError; | ||
| fileResults: SingleAttachmentValidationResult[]; | ||
| files: FileObject[]; | ||
| }; | ||
| type MultipleAttachmentsValidationResult = MultipleAttachmentsValidResult | MultipleAttachmentsInvalidResult; | ||
|
|
||
| function isMultipleAttachmentsValidationResult(result: unknown): result is MultipleAttachmentsValidationResult { | ||
| return typeof result === 'object' && result !== null && 'isValid' in result && typeof result.isValid === 'boolean' && ('validatedFiles' in result || 'fileResults' in result); | ||
| } | ||
|
|
||
| function validateMultipleAttachmentFiles(files: FileObject[], items?: DataTransferItem[], isValidatingReceipts = false): Promise<MultipleAttachmentsValidationResult> { | ||
| if (!files?.length || files.some((f) => isDirectory(f))) { | ||
| return Promise.resolve({isValid: false, error: CONST.FILE_VALIDATION_ERRORS.MULTIPLE_FILES.FOLDER_NOT_ALLOWED, fileResults: [], files}); | ||
| } | ||
|
|
||
| if (files.length > CONST.API_ATTACHMENT_VALIDATIONS.MAX_FILE_LIMIT) { | ||
| return Promise.resolve({isValid: false, error: CONST.FILE_VALIDATION_ERRORS.MULTIPLE_FILES.MAX_FILE_LIMIT_EXCEEDED, fileResults: [], files}); | ||
| } | ||
|
|
||
| return Promise.all(files.map((f, index) => validateAttachmentFile(f, items?.at(index), isValidatingReceipts))).then((results) => { | ||
| if (results.every((result) => result.isValid)) { | ||
| return { | ||
| isValid: true, | ||
| validatedFiles: results.map((result) => result.validatedFile), | ||
| }; | ||
| } | ||
|
|
||
| return { | ||
| isValid: false, | ||
| error: CONST.FILE_VALIDATION_ERRORS.MULTIPLE_FILES.WRONG_FILE_TYPE, | ||
| fileResults: results, | ||
| files, | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| function isFileCorrupted(fileObject: FileObject): Promise<SingleAttachmentValidationResult> { | ||
| return normalizeFileObject(fileObject).then((normalizedFile) => { | ||
| return validateImageForCorruption(normalizedFile) | ||
| .then(() => { | ||
| if (normalizedFile.size && normalizedFile.size > CONST.API_ATTACHMENT_VALIDATIONS.MAX_SIZE) { | ||
| return { | ||
| isValid: false, | ||
| error: CONST.FILE_VALIDATION_ERRORS.SINGLE_FILE.FILE_TOO_LARGE, | ||
| } as SingleAttachmentInvalidResult; | ||
| } | ||
|
|
||
| if (normalizedFile.size && normalizedFile.size < CONST.API_ATTACHMENT_VALIDATIONS.MIN_SIZE) { | ||
| return { | ||
| isValid: false, | ||
| error: CONST.FILE_VALIDATION_ERRORS.SINGLE_FILE.FILE_TOO_SMALL, | ||
| } as SingleAttachmentInvalidResult; | ||
| } | ||
|
|
||
| return { | ||
| isValid: true, | ||
| } as SingleAttachmentValidResult; | ||
| }) | ||
| .catch(() => { | ||
| return { | ||
| isValid: false, | ||
| error: CONST.FILE_VALIDATION_ERRORS.SINGLE_FILE.FILE_INVALID, | ||
| } as SingleAttachmentInvalidResult; | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| function isDirectory(data: FileObject) { | ||
| if ('webkitGetAsEntry' in data && (data as DataTransferItem).webkitGetAsEntry()?.isDirectory) { | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| export {validateAttachmentFile, validateMultipleAttachmentFiles, isSingleAttachmentValidationResult, isMultipleAttachmentsValidationResult}; | ||
| export type { | ||
| SingleAttachmentValidationResult, | ||
| SingleAttachmentValidResult, | ||
| SingleAttachmentInvalidResult, | ||
| SingleAttachmentValidationError, | ||
| MultipleAttachmentsValidationResult, | ||
| MultipleAttachmentsValidResult, | ||
| MultipleAttachmentsInvalidResult, | ||
| MultipleAttachmentsValidationError, | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.