diff --git a/src/client/javascripts/file-upload.js b/src/client/javascripts/file-upload.js index c19e0429b..2ae67ad9d 100644 --- a/src/client/javascripts/file-upload.js +++ b/src/client/javascripts/file-upload.js @@ -304,16 +304,19 @@ function pollUploadStatus(uploadId) { * @param {HTMLInputElement} fileInput - The file input element * @param {HTMLButtonElement} uploadButton - The upload button * @param {HTMLButtonElement} continueButton - The continue button - * @param {File | null} selectedFile - The selected file + * @param {File[]} selectedFiles - The selected files */ function handleStandardFormSubmission( formElement, fileInput, uploadButton, continueButton, - selectedFile + selectedFiles ) { - renderSummary(selectedFile, 'Uploading…', formElement) + // Render in reverse so first file ends up at the top of the summary list + for (let i = selectedFiles.length - 1; i >= 0; i--) { + renderSummary(selectedFiles[i], 'Uploading…', formElement) + } fileInput.focus() @@ -403,8 +406,8 @@ function initUpload() { } const formElement = /** @type {HTMLFormElement} */ (form) - /** @type {File | null} */ - let selectedFile = null + /** @type {File[]} */ + let selectedFiles = [] let isSubmitting = false const uploadId = formElement.dataset.uploadId @@ -414,12 +417,12 @@ function initUpload() { } if (fileInput.files && fileInput.files.length > 0) { - selectedFile = fileInput.files[0] + selectedFiles = Array.from(fileInput.files) } }) uploadButton.addEventListener('click', (event) => { - if (!selectedFile) { + if (selectedFiles.length === 0) { event.preventDefault() showError( 'Select a file', @@ -436,12 +439,13 @@ function initUpload() { isSubmitting = true + // Show all selected files in the summary table handleStandardFormSubmission( formElement, fileInput, uploadButton, continueButton, - selectedFile + selectedFiles ) handleAjaxFormSubmission( diff --git a/src/server/plugins/engine/components/FileUploadField.test.ts b/src/server/plugins/engine/components/FileUploadField.test.ts index 870f75160..3e91ef63f 100644 --- a/src/server/plugins/engine/components/FileUploadField.test.ts +++ b/src/server/plugins/engine/components/FileUploadField.test.ts @@ -405,7 +405,7 @@ describe('FileUploadField', () => { actions: { items: [ { - href: `/test/file-upload-component/${validState[0].uploadId}/confirm-delete`, + href: `/test/file-upload-component/${validState[0].status.form.file.fileId}/confirm-delete`, text: 'Remove', attributes: { id: 'myComponent__0' }, classes: 'govuk-link--no-visited-state', @@ -424,7 +424,7 @@ describe('FileUploadField', () => { actions: { items: [ { - href: `/test/file-upload-component/${validState[1].uploadId}/confirm-delete`, + href: `/test/file-upload-component/${validState[1].status.form.file.fileId}/confirm-delete`, text: 'Remove', attributes: { id: 'myComponent__1' }, classes: 'govuk-link--no-visited-state', @@ -443,7 +443,7 @@ describe('FileUploadField', () => { actions: { items: [ { - href: `/test/file-upload-component/${validState[2].uploadId}/confirm-delete`, + href: `/test/file-upload-component/${validState[2].status.form.file.fileId}/confirm-delete`, text: 'Remove', attributes: { id: 'myComponent__2' }, classes: 'govuk-link--no-visited-state', @@ -454,7 +454,8 @@ describe('FileUploadField', () => { } ] } - } + }, + multiple: true }) ) }) @@ -543,7 +544,7 @@ describe('FileUploadField', () => { actions: { items: [ { - href: `/test/file-upload-component/${validState[2].uploadId}/confirm-delete`, + href: `/test/file-upload-component/${validState[2].status.form.file.fileId}/confirm-delete`, text: 'Remove', attributes: { id: 'myComponent__0' }, classes: 'govuk-link--no-visited-state', @@ -554,7 +555,8 @@ describe('FileUploadField', () => { } ] } - } + }, + multiple: true }) ) }) @@ -583,7 +585,7 @@ describe('FileUploadField', () => { actions: { items: [ { - href: `/test/file-upload-component/${validState[2].uploadId}/confirm-delete`, + href: `/test/file-upload-component/${validState[2].status.form.file.fileId}/confirm-delete`, text: 'Remove', attributes: { id: 'myComponent__0' }, classes: 'govuk-link--no-visited-state', @@ -594,7 +596,8 @@ describe('FileUploadField', () => { } ] } - } + }, + multiple: true }) ) }) diff --git a/src/server/plugins/engine/components/FileUploadField.ts b/src/server/plugins/engine/components/FileUploadField.ts index c1696167f..4144600d9 100644 --- a/src/server/plugins/engine/components/FileUploadField.ts +++ b/src/server/plugins/engine/components/FileUploadField.ts @@ -73,9 +73,12 @@ export const tempStatusSchema = joi .valid(UploadStatus.ready, UploadStatus.pending) .required(), metadata: metadataSchema, - form: joi.object().required().keys({ - file: tempFileSchema - }), + form: joi + .object() + .required() + .keys({ + file: joi.array().items(tempFileSchema).single().required() + }), numberOfRejectedFiles: joi.number().optional() }) .required() @@ -191,7 +194,7 @@ export class FileUploadField extends FormComponent { errors?: FormSubmissionError[], query: FormQuery = {} ) { - const { options, page } = this + const { options, page, schema } = this // Allow preview URL direct access const isForceAccess = 'force' in query @@ -233,7 +236,7 @@ export class FileUploadField extends FormComponent { // Remove summary list actions from previews if (!isForceAccess) { - const path = `/${item.uploadId}/confirm-delete` + const path = `/${file.fileId}/confirm-delete` const href = page?.getHref(`${page.path}${path}`) ?? '#' items.push({ @@ -263,6 +266,9 @@ export class FileUploadField extends FormComponent { attributes.accept = options.accept } + // Allow multiple file selection when schema permits more than 1 file + const allowsMultiple = schema.max !== 1 && schema.length !== 1 + const summaryList: SummaryList = { classes: 'govuk-summary-list--long-key', rows @@ -277,6 +283,9 @@ export class FileUploadField extends FormComponent { // Override the component name we send to CDP name: 'file', + // Enable multi-file selection in the file picker + ...(allowsMultiple && { multiple: true }), + upload: { count, summaryList diff --git a/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts b/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts index 393ba30a1..6d2060ccc 100644 --- a/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts +++ b/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts @@ -859,7 +859,16 @@ describe('FileUploadPageController', () => { describe('file removal', () => { it('returns early when no file is removed', async () => { - const files = [{ uploadId: 'file1' }, { uploadId: 'file2' }] + const files = [ + { + uploadId: 'upload1', + status: { form: { file: { fileId: 'file1' } } } + }, + { + uploadId: 'upload2', + status: { form: { file: { fileId: 'file2' } } } + } + ] Object.defineProperty(request, 'params', { value: { itemId: 'nonexistent-file' }, @@ -892,7 +901,16 @@ describe('FileUploadPageController', () => { }) it('merges state when file is removed', async () => { - const files = [{ uploadId: 'file1' }, { uploadId: 'file2' }] + const files = [ + { + uploadId: 'upload1', + status: { form: { file: { fileId: 'file1' } } } + }, + { + uploadId: 'upload2', + status: { form: { file: { fileId: 'file2' } } } + } + ] Object.defineProperty(request, 'params', { value: { itemId: 'file1' }, @@ -924,7 +942,12 @@ describe('FileUploadPageController', () => { expect(mergeStateSpy).toHaveBeenCalledWith(request, state, { upload: { [controller.path]: { - files: [{ uploadId: 'file2' }], + files: [ + { + uploadId: 'upload2', + status: { form: { file: { fileId: 'file2' } } } + } + ], upload: { uploadId: 'upload-123', uploadUrl: 'some-url', @@ -1121,11 +1144,15 @@ describe('FileUploadPageController', () => { files: [ { uploadId: 'file-1', - status: { form: { file: { filename: 'file-1.pdf' } } } + status: { + form: { file: { fileId: 'file-1', filename: 'file-1.pdf' } } + } }, { uploadId: 'file-2', - status: { form: { file: { filename: 'file-2.pdf' } } } + status: { + form: { file: { fileId: 'file-2', filename: 'file-2.pdf' } } + } } ] } diff --git a/src/server/plugins/engine/pageControllers/FileUploadPageController.ts b/src/server/plugins/engine/pageControllers/FileUploadPageController.ts index 1a59a3e01..7fef3977c 100644 --- a/src/server/plugins/engine/pageControllers/FileUploadPageController.ts +++ b/src/server/plugins/engine/pageControllers/FileUploadPageController.ts @@ -27,6 +27,7 @@ import { type AnyFormRequest, type FeaturedFormPageViewModel, type FileState, + type FileUpload, type FormContext, type FormContextRequest, type FormSubmissionError, @@ -177,7 +178,7 @@ export class FileUploadPageController extends QuestionPageController { const files = this.getFilesFromState(state) const fileToRemove = files.find( - ({ uploadId }) => uploadId === params.itemId + ({ status }) => status.form.file.fileId === params.itemId ) if (!fileToRemove) { @@ -385,39 +386,84 @@ export class FileUploadPageController extends QuestionPageController { // Only add to files state if the file validates. // This secures against html tampering of the file input - // by adding a 'multiple' attribute or it being - // changed to a simple text field or similar. + // (e.g. changing it to a simple text field or similar). const validationResult = tempItemSchema.validate( { uploadId, status: statusResponse }, { stripUnknown: true } ) const error = validationResult.error - const fileState = validationResult.value as FileState if (error) { return this.initiateAndStoreNewUpload(request, state) } - const file = fileState.status.form.file - if (file.fileStatus === FileStatus.complete) { - files.unshift(prepareFileState(fileState)) + // CDP returns form.file as a single object for one file, + // or an array for multiple files. The Joi schema normalises + // both to an array via .single(). + await this.processUploadedFiles( + request, + state, + validationResult.value, + files, + upload + ) + + return this.initiateAndStoreNewUpload(request, state) + } + + /** + * Processes the uploaded files from a CDP status response. + * Complete files are added to state, rejected/pending files + * have their error messages flashed. + * @param request - the hapi request + * @param state - the form state + * @param validatedItem - the Joi-validated upload item + * @param files - the current files array from state + * @param upload - the current upload initiation response + */ + private async processUploadedFiles( + request: AnyFormRequest, + state: FormSubmissionState, + validatedItem: FileState, + files: FileState[], + upload: UploadInitiateResponse | undefined + ) { + const { uploadId } = validatedItem + const validatedStatus = validatedItem.status + const rawFile = validatedStatus.form.file as unknown as + | FileUpload + | FileUpload[] + const uploadedFiles = Array.isArray(rawFile) ? rawFile : [rawFile] + + for (const file of uploadedFiles) { + if (file.fileStatus === FileStatus.complete) { + const perFileState: FileState = { + uploadId, + status: { + ...validatedStatus, + form: { file } + } as FileState['status'] + } + files.unshift(prepareFileState(perFileState)) + } else { + // Flash the error message for rejected/pending files. + const { fileUpload } = this + const cacheService = getCacheService(request.server) + + const name = fileUpload.name + const text = file.errorMessage ?? 'Unknown error' + const errors: FormSubmissionError[] = [ + { path: [name], href: `#${name}`, name, text } + ] + cacheService.setFlash(request, { errors }) + } + } + + if (uploadedFiles.some((f) => f.fileStatus === FileStatus.complete)) { await this.mergeState(request, state, { upload: { [this.path]: { files, upload } } }) - } else { - // Flash the error message. - const { fileUpload } = this - const cacheService = getCacheService(request.server) - - const name = fileUpload.name - const text = file.errorMessage ?? 'Unknown error' - const errors: FormSubmissionError[] = [ - { path: [name], href: `#${name}`, name, text } - ] - cacheService.setFlash(request, { errors }) } - - return this.initiateAndStoreNewUpload(request, state) } /** @@ -438,7 +484,7 @@ export class FileUploadPageController extends QuestionPageController { const files = this.getFilesFromState(state) const filesUpdated = files.filter( - ({ uploadId }) => uploadId !== params.itemId + ({ status }) => status.form.file.fileId !== params.itemId ) if (filesUpdated.length === files.length) { diff --git a/test/form/file-upload.test.js b/test/form/file-upload.test.js index cb9a08507..7d84a7288 100644 --- a/test/form/file-upload.test.js +++ b/test/form/file-upload.test.js @@ -295,14 +295,14 @@ describe('File upload POST tests', () => { expect(res2.statusCode).toBe(StatusCodes.OK) const res3 = await server.inject({ - url: `${basePath}/methodology-statement/15b2303c-9965-4632-acb6-0776081e0399/confirm-delete`, + url: `${basePath}/methodology-statement/5a76a1a3-bc8a-4bc0-859a-116d775c7f15/confirm-delete`, headers }) expect(res3.statusCode).toBe(StatusCodes.OK) const res4 = await server.inject({ - url: `${basePath}/methodology-statement/15b2303c-9965-4632-acb6-0776081e0399/confirm-delete`, + url: `${basePath}/methodology-statement/5a76a1a3-bc8a-4bc0-859a-116d775c7f15/confirm-delete`, method: 'POST', headers, payload: {