Skip to content

Commit

Permalink
Support large(r) file uploads by directly uploading to S3 (#2012)
Browse files Browse the repository at this point in the history 
* update client code to get upload url, post to upload, post to finalize

* restrict all cvr files uploads to 1 file

* client tests passing

* add file helper and parsing utils

* update server endpoints to break upload into 3 steps

* handle unwrapped hart zip

* add helper functions to manage test uploads

* add tests for public file upload endpoint

* add tests for get upload url endpoints

* refactor server tests to call helper functions to upload files and handle error testing

* update test missed in rebase

* change files to file in client useCSV

* update upload complete calls to use json not form data

* nit changes to file utils

* dont create subdir for cvrs

* make upload file require logged in user

* address various pr nit comments in api

* rename test helpers

* fix server to expect json params on upload complete

* test csp on server
  • Loading branch information
@carolinemodic
carolinemodic authored Oct 30, 2024
1 parent 7a20882 commit 699b659
Show file tree
Hide file tree
Showing 54 changed files with 3,431 additions and 2,727 deletions.
35 changes: 15 additions & 20 deletions client/src/components/Atoms/CSVForm.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { CvrFileType, IFileInfo, FileProcessingStatus } from '../useCSV'
import FormWrapper from './Form/FormWrapper'
import { FormSectionDescription } from './Form/FormSection'
import { ErrorLabel, SuccessLabel } from './Form/_helpers'
import { sum } from '../../utils/number'
import FormButton from './Form/FormButton'
import AsyncButton from './AsyncButton'

Expand All @@ -30,13 +29,13 @@ const schema = Yup.object().shape({
})

interface IValues {
csv: File[] | null
csv: File | null
cvrFileType?: CvrFileType
}

interface IProps {
csvFile: IFileInfo
uploadCSVFiles: (csvs: File[], cvrFileType?: CvrFileType) => Promise<boolean>
uploadCSVFile: (csv: File, cvrFileType?: CvrFileType) => Promise<boolean>
deleteCSVFile?: () => Promise<boolean>
title?: string
description: string
Expand All @@ -47,7 +46,7 @@ interface IProps {

const CSVFile: React.FC<IProps> = ({
csvFile,
uploadCSVFiles,
uploadCSVFile,
deleteCSVFile,
title,
description,
Expand All @@ -62,7 +61,7 @@ const CSVFile: React.FC<IProps> = ({
return (
<Formik
initialValues={{
csv: isProcessing ? [new File([], file!.name)] : null,
csv: isProcessing ? new File([], file!.name) : null,
cvrFileType: showCvrFileType
? file
? file.cvrFileType
Expand All @@ -74,7 +73,7 @@ const CSVFile: React.FC<IProps> = ({
validateOnBlur={false}
onSubmit={async (values: IValues) => {
if (values.csv) {
await uploadCSVFiles(values.csv, values.cvrFileType)
await uploadCSVFile(values.csv, values.cvrFileType)
setIsEditing(false)
}
}}
Expand Down Expand Up @@ -117,34 +116,30 @@ const CSVFile: React.FC<IProps> = ({
<FileInput
inputProps={{
// While this component is named CSVFile, it can accept zip files in the case
// of Hart CVRs
// of Hart and ESS CVRs
// TODO: Consider renaming the component and its internals accordingly
accept:
values.cvrFileType === CvrFileType.HART
? '.zip,.csv'
values.cvrFileType &&
[CvrFileType.HART, CvrFileType.ESS].includes(
values.cvrFileType
)
? '.zip'
: '.csv',
name: 'csv',
multiple:
values.cvrFileType === CvrFileType.ESS ||
values.cvrFileType === CvrFileType.HART,
}}
onInputChange={e => {
const { files } = e.currentTarget
setFieldValue(
'csv',
files && files.length > 0 ? Array.from(files) : null
files && files.length === 1 ? files[0] : null
)
}}
hasSelection={!!values.csv}
text={(() => {
if (!values.csv) {
return values.cvrFileType === CvrFileType.ESS ||
values.cvrFileType === CvrFileType.HART
? 'Select files...'
: 'Select a file...'
return 'Select a file...'
}
if (values.csv.length === 1) return values.csv[0].name
return `${values.csv.length} files selected`
return values.csv.name
})()}
onBlur={handleBlur}
disabled={isSubmitting || isProcessing || !enabled}
Expand Down Expand Up @@ -180,7 +175,7 @@ const CSVFile: React.FC<IProps> = ({
{upload &&
// Only show upload progress for large sets of files (over 1 MB),
// otherwise it will just flash on the screen
sum(upload.files.map(f => f.size)) >= 1000 * 1000 && (
upload.file.size >= 1000 * 1000 && (
<>
<span style={{ marginRight: '5px' }}>
Uploading...
Expand Down
176 changes: 157 additions & 19 deletions client/src/components/Atoms/FileUpload.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,7 @@ const TestFileUpload = ({
const fileUpload: IFileUpload = {
uploadedFile,
uploadFiles: files => {
const formData = new FormData()
for (const file of files) {
formData.append('files', file, file.name)
}
return uploadFiles.mutateAsync(formData)
return uploadFiles.mutateAsync({ file: files[0] })
},
uploadProgress: uploadFiles.progress,
deleteFile: () => deleteFile.mutateAsync(),
Expand All @@ -66,16 +62,50 @@ const render = (element: React.ReactElement) =>
const testFile = new File(['test content'], 'test-file.csv', {
type: 'text/csv',
})
const formData = new FormData()
formData.append('files', testFile, testFile.name)
const uploadFormData = new FormData()
uploadFormData.append('key', 'path/to/file/file.csv')
uploadFormData.append('otherField', 'canBePassedThrough')
uploadFormData.append('Content-Type', testFile.type)
uploadFormData.append('file', testFile, testFile.name)

const uploadCompleteJSONData = {
fileName: testFile.name,
fileType: testFile.type,
storagePathKey: 'path/to/file/file.csv',
}

const getUploadUrlMock = {
url: '/test/file-upload',
fields: {
key: 'path/to/file/file.csv',
otherField: 'canBePassedThrough',
},
}

describe('FileUpload + useFileUpload', () => {
it('when no file is uploaded, shows a form to upload a file', async () => {
const expectedCalls = [
{ url: '/test', response: fileInfoMocks.empty },
{
url: '/test',
options: { method: 'PUT', body: formData },
url: '/test/upload-url',
options: {
method: 'GET',
params: { fileType: testFile.type },
},
response: getUploadUrlMock,
},
{
url: '/test/file-upload',
options: { method: 'POST', body: uploadFormData },
response: { status: 'ok' },
},
{
url: '/test/upload-complete',
options: {
method: 'POST',
body: (uploadCompleteJSONData as unknown) as BodyInit,
headers: { 'Content-Type': 'application/json' },
},
response: { status: 'ok' },
},
{ url: '/test', response: fileInfoMocks.processing },
Expand Down Expand Up @@ -138,8 +168,25 @@ describe('FileUpload + useFileUpload', () => {
const expectedCalls = [
{ url: '/test', response: fileInfoMocks.empty },
{
url: '/test',
options: { method: 'PUT', body: formData },
url: '/test/upload-url',
options: {
method: 'GET',
params: { fileType: testFile.type },
},
response: getUploadUrlMock,
},
{
url: '/test/file-upload',
options: { method: 'POST', body: uploadFormData },
response: { status: 'ok' },
},
{
url: '/test/upload-complete',
options: {
method: 'POST',
body: (uploadCompleteJSONData as unknown) as BodyInit,
headers: { 'Content-Type': 'application/json' },
},
response: { status: 'ok' },
},
{ url: '/test', response: fileInfoMocks.errored },
Expand Down Expand Up @@ -175,8 +222,25 @@ describe('FileUpload + useFileUpload', () => {
const expectedCalls = [
{ url: '/test', response: fileInfoMocks.empty },
{
url: '/test',
options: { method: 'PUT', body: formData2 },
url: '/test/upload-url',
options: {
method: 'GET',
params: { fileType: testFile.type },
},
response: getUploadUrlMock,
},
{
url: '/test/file-upload',
options: { method: 'POST', body: uploadFormData },
response: { status: 'ok' },
},
{
url: '/test/upload-complete',
options: {
method: 'POST',
body: (uploadCompleteJSONData as unknown) as BodyInit,
headers: { 'Content-Type': 'application/json' },
},
response: { status: 'ok' },
},
{ url: '/test', response: fileInfoMocks.processed },
Expand Down Expand Up @@ -232,14 +296,88 @@ describe('FileUpload + useFileUpload', () => {
})
})

it('handles an API error on put', async () => {
it('handles an API error on get', async () => {
const expectedCalls = [
{ url: '/test', response: fileInfoMocks.empty },
serverError('putFile', {
url: '/test',
serverError('getFile', {
url: '/test/upload-url',
options: {
method: 'GET',
params: { fileType: testFile.type },
} as RequestInit,
}),
]
await withMockFetch(expectedCalls, async () => {
render(
<>
<TestFileUpload />
<ToastContainer />
</>
)
await screen.findByText('Test File')
userEvent.upload(screen.getByLabelText('Select a file...'), testFile)
await screen.findByText('test-file.csv')
userEvent.click(screen.getByRole('button', { name: /Upload/ }))
await findAndCloseToast('getFile')
})
})

it('handles an API error on file upload', async () => {
const expectedCalls = [
{ url: '/test', response: fileInfoMocks.empty },
{
url: '/test/upload-url',
options: {
method: 'GET',
params: { fileType: testFile.type },
},
response: getUploadUrlMock,
},
serverError('postFileUpload', {
url: '/test/file-upload',
options: {
method: 'POST',
body: uploadFormData,
},
}),
]
await withMockFetch(expectedCalls, async () => {
render(
<>
<TestFileUpload />
<ToastContainer />
</>
)
await screen.findByText('Test File')
userEvent.upload(screen.getByLabelText('Select a file...'), testFile)
await screen.findByText('test-file.csv')
userEvent.click(screen.getByRole('button', { name: /Upload/ }))
await findAndCloseToast('postFileUpload')
})
})

it('handles an API error on file upload completion', async () => {
const expectedCalls = [
{ url: '/test', response: fileInfoMocks.empty },
{
url: '/test/upload-url',
options: {
method: 'GET',
params: { fileType: testFile.type },
},
response: getUploadUrlMock,
},
{
url: '/test/file-upload',
options: { method: 'POST', body: uploadFormData },
response: { status: 'ok' },
},
serverError('postFileComplete', {
url: '/test/upload-complete',
options: {
method: 'PUT',
body: formData,
method: 'POST',
body: (uploadCompleteJSONData as unknown) as BodyInit,
headers: { 'Content-Type': 'application/json' },
},
}),
]
Expand All @@ -254,7 +392,7 @@ describe('FileUpload + useFileUpload', () => {
userEvent.upload(screen.getByLabelText('Select a file...'), testFile)
await screen.findByText('test-file.csv')
userEvent.click(screen.getByRole('button', { name: /Upload/ }))
await findAndCloseToast('putFile')
await findAndCloseToast('postFileComplete')
})
})

Expand Down
14 changes: 10 additions & 4 deletions client/src/components/AuditAdmin/AuditAdminView.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,9 @@ describe('AA setup flow', () => {
aaApiCalls.getContests(contestMocks.filledTargeted),
aaApiCalls.getSettings(auditSettingsMocks.all),
aaApiCalls.getJurisdictionFileWithResponse(jurisdictionFileMocks.empty),
aaApiCalls.putJurisdictionFile,
aaApiCalls.uploadJurisdictionFileGetUrl,
aaApiCalls.uploadJurisdictionFilePostFile,
aaApiCalls.uploadJurisdictionFileUploadComplete,
aaApiCalls.getJurisdictionFileWithResponse(
jurisdictionFileMocks.processed
),
Expand Down Expand Up @@ -160,7 +162,9 @@ describe('AA setup flow', () => {
aaApiCalls.getContests(contestMocks.filledTargeted),
aaApiCalls.getSettings(auditSettingsMocks.all),
aaApiCalls.getJurisdictionFileWithResponse(jurisdictionFileMocks.empty),
aaApiCalls.putJurisdictionErrorFile,
aaApiCalls.uploadJurisdictionFileGetUrl,
aaApiCalls.uploadJurisdictionFilePostFile,
aaApiCalls.uploadJurisdictionFileUploadCompleteError,
aaApiCalls.getJurisdictionFileWithResponse(jurisdictionFileMocks.errored),
aaApiCalls.getJurisdictions,
]
Expand Down Expand Up @@ -197,7 +201,9 @@ describe('AA setup flow', () => {
aaApiCalls.getStandardizedContestsFileWithResponse(
standardizedContestsFileMocks.empty
),
aaApiCalls.putStandardizedContestsFile,
aaApiCalls.uploadStandardizedContestsFileGetUrl,
aaApiCalls.uploadStandardizedContestsFilePostFile,
aaApiCalls.uploadStandardizedContestsFileUploadComplete,
aaApiCalls.getStandardizedContestsFileWithResponse(
standardizedContestsFileMocks.processed
),
Expand Down Expand Up @@ -320,7 +326,7 @@ describe('AA setup flow', () => {
aaApiCalls.getSettings(auditSettingsMocks.all),
aaApiCalls.getMapData,
jaApiCalls.getBallotManifestFile(manifestMocks.empty),
jaApiCalls.putManifest,
...jaApiCalls.uploadManifestCalls,
jaApiCalls.getBallotManifestFile(manifestMocks.processed),
{
...aaApiCalls.getJurisdictions,
Expand Down
Loading

0 comments on commit 699b659

Please sign in to comment.