Skip to content

Commit 61d0a39

Browse files
mokhldmokhld
authored
feat(DF-868): multiple uploads (#339)
* feat(DF-868): enable multiple file selection in upload field Allow multi-file selection when the component schema permits it, and render all selected files in the client-side upload summary. * feat(DF-868): handle multiple uploaded files in controller Normalise CDP status to support multiple uploaded files, key file removal by fileId, and update tests to match. * refactor: reduce function length * fix(DF-868): collect all file errors into single flash message Prevents loss of error messages when multiple files fail upload.
1 parent a71a8c5 commit 61d0a39

File tree

6 files changed

+217
-49
lines changed

6 files changed

+217
-49
lines changed

‎src/client/javascripts/file-upload.js‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -304,16 +304,19 @@ function pollUploadStatus(uploadId) {
304304
* @param {HTMLInputElement} fileInput - The file input element
305305
* @param {HTMLButtonElement} uploadButton - The upload button
306306
* @param {HTMLButtonElement} continueButton - The continue button
307-
* @param {File | null} selectedFile - The selected file
307+
* @param {File[]} selectedFiles - The selected files
308308
*/
309309
function handleStandardFormSubmission(
310310
formElement,
311311
fileInput,
312312
uploadButton,
313313
continueButton,
314-
selectedFile
314+
selectedFiles
315315
) {
316-
renderSummary(selectedFile, 'Uploading…', formElement)
316+
// Render in reverse so first file ends up at the top of the summary list
317+
for (let i = selectedFiles.length - 1; i >= 0; i--) {
318+
renderSummary(selectedFiles[i], 'Uploading…', formElement)
319+
}
317320

318321
fileInput.focus()
319322

@@ -403,8 +406,8 @@ function initUpload() {
403406
}
404407

405408
const formElement = /** @type {HTMLFormElement} */ (form)
406-
/** @type {File | null} */
407-
let selectedFile = null
409+
/** @type {File[]} */
410+
let selectedFiles = []
408411
let isSubmitting = false
409412
const uploadId = formElement.dataset.uploadId
410413

@@ -414,12 +417,12 @@ function initUpload() {
414417
}
415418

416419
if (fileInput.files && fileInput.files.length > 0) {
417-
selectedFile = fileInput.files[0]
420+
selectedFiles = Array.from(fileInput.files)
418421
}
419422
})
420423

421424
uploadButton.addEventListener('click', (event) => {
422-
if (!selectedFile) {
425+
if (selectedFiles.length === 0) {
423426
event.preventDefault()
424427
showError(
425428
'Select a file',
@@ -436,12 +439,13 @@ function initUpload() {
436439

437440
isSubmitting = true
438441

442+
// Show all selected files in the summary table
439443
handleStandardFormSubmission(
440444
formElement,
441445
fileInput,
442446
uploadButton,
443447
continueButton,
444-
selectedFile
448+
selectedFiles
445449
)
446450

447451
handleAjaxFormSubmission(

‎src/server/plugins/engine/components/FileUploadField.test.ts‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ describe('FileUploadField', () => {
405405
actions: {
406406
items: [
407407
{
408-
href: `/test/file-upload-component/${validState[0].uploadId}/confirm-delete`,
408+
href: `/test/file-upload-component/${validState[0].status.form.file.fileId}/confirm-delete`,
409409
text: 'Remove',
410410
attributes: { id: 'myComponent__0' },
411411
classes: 'govuk-link--no-visited-state',
@@ -424,7 +424,7 @@ describe('FileUploadField', () => {
424424
actions: {
425425
items: [
426426
{
427-
href: `/test/file-upload-component/${validState[1].uploadId}/confirm-delete`,
427+
href: `/test/file-upload-component/${validState[1].status.form.file.fileId}/confirm-delete`,
428428
text: 'Remove',
429429
attributes: { id: 'myComponent__1' },
430430
classes: 'govuk-link--no-visited-state',
@@ -443,7 +443,7 @@ describe('FileUploadField', () => {
443443
actions: {
444444
items: [
445445
{
446-
href: `/test/file-upload-component/${validState[2].uploadId}/confirm-delete`,
446+
href: `/test/file-upload-component/${validState[2].status.form.file.fileId}/confirm-delete`,
447447
text: 'Remove',
448448
attributes: { id: 'myComponent__2' },
449449
classes: 'govuk-link--no-visited-state',
@@ -454,7 +454,8 @@ describe('FileUploadField', () => {
454454
}
455455
]
456456
}
457-
}
457+
},
458+
multiple: true
458459
})
459460
)
460461
})
@@ -543,7 +544,7 @@ describe('FileUploadField', () => {
543544
actions: {
544545
items: [
545546
{
546-
href: `/test/file-upload-component/${validState[2].uploadId}/confirm-delete`,
547+
href: `/test/file-upload-component/${validState[2].status.form.file.fileId}/confirm-delete`,
547548
text: 'Remove',
548549
attributes: { id: 'myComponent__0' },
549550
classes: 'govuk-link--no-visited-state',
@@ -554,7 +555,8 @@ describe('FileUploadField', () => {
554555
}
555556
]
556557
}
557-
}
558+
},
559+
multiple: true
558560
})
559561
)
560562
})
@@ -583,7 +585,7 @@ describe('FileUploadField', () => {
583585
actions: {
584586
items: [
585587
{
586-
href: `/test/file-upload-component/${validState[2].uploadId}/confirm-delete`,
588+
href: `/test/file-upload-component/${validState[2].status.form.file.fileId}/confirm-delete`,
587589
text: 'Remove',
588590
attributes: { id: 'myComponent__0' },
589591
classes: 'govuk-link--no-visited-state',
@@ -594,7 +596,8 @@ describe('FileUploadField', () => {
594596
}
595597
]
596598
}
597-
}
599+
},
600+
multiple: true
598601
})
599602
)
600603
})

‎src/server/plugins/engine/components/FileUploadField.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,12 @@ export const tempStatusSchema = joi
7373
.valid(UploadStatus.ready, UploadStatus.pending)
7474
.required(),
7575
metadata: metadataSchema,
76-
form: joi.object().required().keys({
77-
file: tempFileSchema
78-
}),
76+
form: joi
77+
.object()
78+
.required()
79+
.keys({
80+
file: joi.array().items(tempFileSchema).single().required()
81+
}),
7982
numberOfRejectedFiles: joi.number().optional()
8083
})
8184
.required()
@@ -191,7 +194,7 @@ export class FileUploadField extends FormComponent {
191194
errors?: FormSubmissionError[],
192195
query: FormQuery = {}
193196
) {
194-
const { options, page } = this
197+
const { options, page, schema } = this
195198

196199
// Allow preview URL direct access
197200
const isForceAccess = 'force' in query
@@ -233,7 +236,7 @@ export class FileUploadField extends FormComponent {
233236

234237
// Remove summary list actions from previews
235238
if (!isForceAccess) {
236-
const path = `/${item.uploadId}/confirm-delete`
239+
const path = `/${file.fileId}/confirm-delete`
237240
const href = page?.getHref(`${page.path}${path}`) ?? '#'
238241

239242
items.push({
@@ -263,6 +266,9 @@ export class FileUploadField extends FormComponent {
263266
attributes.accept = options.accept
264267
}
265268

269+
// Allow multiple file selection when schema permits more than 1 file
270+
const allowsMultiple = schema.max !== 1 && schema.length !== 1
271+
266272
const summaryList: SummaryList = {
267273
classes: 'govuk-summary-list--long-key',
268274
rows
@@ -277,6 +283,9 @@ export class FileUploadField extends FormComponent {
277283
// Override the component name we send to CDP
278284
name: 'file',
279285

286+
// Enable multi-file selection in the file picker
287+
...(allowsMultiple && { multiple: true }),
288+
280289
upload: {
281290
count,
282291
summaryList

‎src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts‎

Lines changed: 109 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,83 @@ describe('FileUploadPageController', () => {
795795
})
796796
})
797797

798+
it('collects all file errors into a single flash when multiple files fail', async () => {
799+
const state = {
800+
upload: {
801+
[controller.path]: {
802+
upload: {
803+
uploadId: 'some-id',
804+
uploadUrl: 'some-url',
805+
statusUrl: 'some-status-url'
806+
},
807+
files: []
808+
}
809+
}
810+
} as unknown as FormSubmissionState
811+
812+
const errorStatus = {
813+
uploadStatus: UploadStatus.ready,
814+
form: {
815+
file: [
816+
{
817+
fileStatus: FileStatus.rejected,
818+
errorMessage: 'File too large'
819+
},
820+
{
821+
fileStatus: FileStatus.rejected,
822+
errorMessage: 'Invalid file type'
823+
}
824+
]
825+
}
826+
}
827+
828+
jest
829+
.spyOn(uploadService, 'getUploadStatus')
830+
.mockResolvedValue(errorStatus as unknown as UploadStatusResponse)
831+
832+
jest.spyOn(tempItemSchema, 'validate').mockReturnValue({
833+
value: {
834+
status: errorStatus,
835+
uploadId: 'some-id'
836+
},
837+
error: undefined
838+
} as ValidationResult)
839+
840+
const testController = controller as TestableFileUploadPageController
841+
842+
const initiateSpy = jest.spyOn(
843+
testController,
844+
'initiateAndStoreNewUpload'
845+
) as jest.SpyInstance<
846+
Promise<FormSubmissionState>,
847+
[FormRequest, FormSubmissionState]
848+
>
849+
850+
initiateSpy.mockResolvedValue(state)
851+
852+
const cacheService = getCacheService(request.server)
853+
854+
await controller['checkUploadStatus'](request, state, 1)
855+
856+
expect(cacheService.setFlash).toHaveBeenCalledTimes(1)
857+
expect(cacheService.setFlash).toHaveBeenCalledWith(request, {
858+
errors: [
859+
{
860+
path: ['fileUpload'],
861+
href: '#fileUpload',
862+
name: 'fileUpload',
863+
text: 'File too large'
864+
},
865+
{
866+
path: ['fileUpload'],
867+
href: '#fileUpload',
868+
name: 'fileUpload',
869+
text: 'Invalid file type'
870+
}
871+
]
872+
})
873+
})
874+
798875
it('sets default error message when none provided', async () => {
799876
const state = {
800877
upload: {
@@ -859,7 +936,16 @@ describe('FileUploadPageController', () => {
859936

860937
describe('file removal', () => {
861938
it('returns early when no file is removed', async () => {
862-
const files = [{ uploadId: 'file1' }, { uploadId: 'file2' }]
939+
const files = [
940+
{
941+
uploadId: 'upload1',
942+
status: { form: { file: { fileId: 'file1' } } }
943+
},
944+
{
945+
uploadId: 'upload2',
946+
status: { form: { file: { fileId: 'file2' } } }
947+
}
948+
]
863949

864950
Object.defineProperty(request, 'params', {
865951
value: { itemId: 'nonexistent-file' },
@@ -892,7 +978,16 @@ describe('FileUploadPageController', () => {
892978
})
893979

894980
it('merges state when file is removed', async () => {
895-
const files = [{ uploadId: 'file1' }, { uploadId: 'file2' }]
981+
const files = [
982+
{
983+
uploadId: 'upload1',
984+
status: { form: { file: { fileId: 'file1' } } }
985+
},
986+
{
987+
uploadId: 'upload2',
988+
status: { form: { file: { fileId: 'file2' } } }
989+
}
990+
]
896991

897992
Object.defineProperty(request, 'params', {
898993
value: { itemId: 'file1' },
@@ -924,7 +1019,12 @@ describe('FileUploadPageController', () => {
9241019
expect(mergeStateSpy).toHaveBeenCalledWith(request, state, {
9251020
upload: {
9261021
[controller.path]: {
927-
files: [{ uploadId: 'file2' }],
1022+
files: [
1023+
{
1024+
uploadId: 'upload2',
1025+
status: { form: { file: { fileId: 'file2' } } }
1026+
}
1027+
],
9281028
upload: {
9291029
uploadId: 'upload-123',
9301030
uploadUrl: 'some-url',
@@ -1121,11 +1221,15 @@ describe('FileUploadPageController', () => {
11211221
files: [
11221222
{
11231223
uploadId: 'file-1',
1124-
status: { form: { file: { filename: 'file-1.pdf' } } }
1224+
status: {
1225+
form: { file: { fileId: 'file-1', filename: 'file-1.pdf' } }
1226+
}
11251227
},
11261228
{
11271229
uploadId: 'file-2',
1128-
status: { form: { file: { filename: 'file-2.pdf' } } }
1230+
status: {
1231+
form: { file: { fileId: 'file-2', filename: 'file-2.pdf' } }
1232+
}
11291233
}
11301234
]
11311235
}

0 commit comments

Comments
 (0)