diff --git a/e2e/cypress/e2e/runner/files.feature b/e2e/cypress/e2e/runner/files.feature new file mode 100644 index 0000000000..d8af51ef01 --- /dev/null +++ b/e2e/cypress/e2e/runner/files.feature @@ -0,0 +1,22 @@ +Feature: File upload fields + + Background: + Given the form "files" exists + And the form "files-show-filenames-enabled" exists + + + Scenario Outline: showFilenamesOnSummaryPage shows "Uploaded" or the filename correctly on summary pages + And I navigate to the "
" form + When I upload the file "passes.png" + Then I see "" + Examples: + | form | summaryValue | + | files | Uploaded | + | files-show-filenames-enabled | passes.png | + + + Scenario: Uploading a file shows "You previously uploaded" message + And I navigate to the "files" form + When I upload the file "passes.png" + And I navigate to "/files/file-one" + Then I see "You previously uploaded the file ‘passes.png’" diff --git a/e2e/cypress/e2e/runner/files.js b/e2e/cypress/e2e/runner/files.js new file mode 100644 index 0000000000..d907a8c5b9 --- /dev/null +++ b/e2e/cypress/e2e/runner/files.js @@ -0,0 +1,6 @@ +import { When } from "@badeball/cypress-cucumber-preprocessor"; + +When("I upload the file {string}", (filename) => { + cy.get("input[type=file]").attachFile(filename); + cy.findByRole("button", { name: /continue/i }).click(); +}); diff --git a/e2e/cypress/fixtures/files-show-filenames-enabled.json b/e2e/cypress/fixtures/files-show-filenames-enabled.json new file mode 100644 index 0000000000..f0e7af8827 --- /dev/null +++ b/e2e/cypress/fixtures/files-show-filenames-enabled.json @@ -0,0 +1,48 @@ +{ + "startPage": "/file-one", + "pages": [ + { + "path": "/file-one", + "components": [ + { + "type": "FileUploadField", + "name": "file1", + "title": "File 1", + "options": { + "required": true + }, + "schema": {} + } + ], + "section": "checkBeforeYouStart", + "next": [ + { + "path": "/summary" + } + ], + "title": "Upload file 1" + }, + { + "path": "/summary", + "controller": "./pages/summary.js", + "title": "Summary", + "components": [], + "next": [] + } + ], + "lists": [], + "sections": [ + { + "name": "checkBeforeYouStart", + "title": "Check before you start" + } + ], + "phaseBanner": {}, + "fees": [], + "payApiKey": "", + "outputs": [], + "declaration": "

All the answers you have provided are true to the best of your knowledge.

", + "version": 2, + "conditions": [], + "showFilenamesOnSummaryPage": true +} diff --git a/e2e/cypress/fixtures/files.json b/e2e/cypress/fixtures/files.json new file mode 100644 index 0000000000..a8b41bcd7c --- /dev/null +++ b/e2e/cypress/fixtures/files.json @@ -0,0 +1,47 @@ +{ + "startPage": "/file-one", + "pages": [ + { + "path": "/file-one", + "components": [ + { + "type": "FileUploadField", + "name": "file1", + "title": "File 1", + "options": { + "required": true + }, + "schema": {} + } + ], + "section": "checkBeforeYouStart", + "next": [ + { + "path": "/summary" + } + ], + "title": "Upload file 1" + }, + { + "path": "/summary", + "controller": "./pages/summary.js", + "title": "Summary", + "components": [], + "next": [] + } + ], + "lists": [], + "sections": [ + { + "name": "checkBeforeYouStart", + "title": "Check before you start" + } + ], + "phaseBanner": {}, + "fees": [], + "payApiKey": "", + "outputs": [], + "declaration": "

All the answers you have provided are true to the best of your knowledge.

", + "version": 2, + "conditions": [] +} diff --git a/model/src/schema/schema.ts b/model/src/schema/schema.ts index 45b2fc520b..ec71847a9d 100644 --- a/model/src/schema/schema.ts +++ b/model/src/schema/schema.ts @@ -327,6 +327,7 @@ export const Schema = joi specialPages: specialPagesSchema.optional(), feeOptions: feeOptionSchema, exitOptions: exitSchema.optional(), + showFilenamesOnSummaryPage: joi.boolean().optional(), }); /** diff --git a/runner/src/server/plugins/engine/models/FormModel.ts b/runner/src/server/plugins/engine/models/FormModel.ts index 61a00ba427..088e49b6a0 100644 --- a/runner/src/server/plugins/engine/models/FormModel.ts +++ b/runner/src/server/plugins/engine/models/FormModel.ts @@ -63,6 +63,7 @@ export class FormModel { specialPages: FormDefinition["specialPages"]; exitOptions?: ExitOptions; allowExit: boolean = false; + showFilenamesOnSummaryPage: boolean = false; constructor(def, options) { const result = Schema.validate(def, { abortEarly: false }); @@ -120,6 +121,8 @@ export class FormModel { this.allowExit = true; } + this.showFilenamesOnSummaryPage = def.showFilenamesOnSummaryPage ?? false; + // @ts-ignore this.pages = def.pages.map((pageDef) => this.makePage(pageDef)); this.startPage = this.pages.find((page) => page.path === def.startPage); diff --git a/runner/src/server/plugins/engine/models/SummaryViewModel.ts b/runner/src/server/plugins/engine/models/SummaryViewModel.ts index fe05c724a5..ddeb52ab58 100644 --- a/runner/src/server/plugins/engine/models/SummaryViewModel.ts +++ b/runner/src/server/plugins/engine/models/SummaryViewModel.ts @@ -158,6 +158,8 @@ export class SummaryViewModel { const items: any[] = []; let sectionState = section ? state[section.name] || {} : state; + sectionState.originalFilenames = state.originalFilenames ?? {}; + const sectionPages = relevantPages.filter( (page) => page.section === section ); @@ -352,7 +354,7 @@ function Item( }); } - return { + const item = { name: component.name, path: page.path, label: component.localisedString(component.title), @@ -365,4 +367,14 @@ function Item( dataType: component.dataType, immutable: component.options.disableChangingFromSummary, }; + + if ( + component.type === "FileUploadField" && + model.showFilenamesOnSummaryPage + ) { + item.filename = + sectionState.originalFilenames[component.name]?.originalFilename; + } + + return item; } diff --git a/runner/src/server/plugins/engine/pageControllers/PageControllerBase.ts b/runner/src/server/plugins/engine/pageControllers/PageControllerBase.ts index 9bb88b832e..71a6a5d236 100644 --- a/runner/src/server/plugins/engine/pageControllers/PageControllerBase.ts +++ b/runner/src/server/plugins/engine/pageControllers/PageControllerBase.ts @@ -611,7 +611,7 @@ export class PageControllerBase { /** * other file related errors.. assuming file fields will be on their own page. This will replace all other errors from the page if not.. */ - if (preHandlerErrors) { + if (preHandlerErrors?.length) { const reformattedErrors: any[] = []; preHandlerErrors.forEach((error) => { const reformatted = error; diff --git a/runner/src/server/plugins/engine/plugin.ts b/runner/src/server/plugins/engine/plugin.ts index ec83858a2f..8fe5a777ae 100644 --- a/runner/src/server/plugins/engine/plugin.ts +++ b/runner/src/server/plugins/engine/plugin.ts @@ -11,6 +11,11 @@ import { FormPayload } from "./types"; import { shouldLogin } from "server/plugins/auth"; import config from "../../config"; import * as exit from "./pluginHandlers/exit"; +import { + getFiles, + handleUpload, + validateContentTypes, +} from "./pluginHandlers/files/prehandlers"; configure([ // Configure Nunjucks to allow rendering of content that is revealed conditionally. @@ -280,15 +285,6 @@ export const plugin = { const { uploadService } = server.services([]); - const handleFiles = (request: HapiRequest, h: HapiResponseToolkit) => { - const { path, id } = request.params; - const model = forms[id]; - const page = model?.pages.find( - (page) => normalisePath(page.path) === normalisePath(path) - ); - return uploadService.handleUploadRequest(request, h, page.pageDef); - }; - //TODO:- Merge with POST /{id}/{path*} route, and move to ./pluginHandlers/id/* const postHandler = async ( request: HapiRequest, @@ -330,7 +326,11 @@ export const plugin = { return h.continue; }, }, - pre: [{ method: handleFiles }], + pre: [ + { method: getFiles, assign: "files" }, + { method: validateContentTypes, assign: "validFiles" }, + { method: handleUpload }, + ], handler: postHandler, }, }); diff --git a/runner/src/server/plugins/engine/pluginHandlers/files/prehandlers/getFiles.ts b/runner/src/server/plugins/engine/pluginHandlers/files/prehandlers/getFiles.ts new file mode 100644 index 0000000000..bae29f4307 --- /dev/null +++ b/runner/src/server/plugins/engine/pluginHandlers/files/prehandlers/getFiles.ts @@ -0,0 +1,18 @@ +import { HapiRequest, HapiResponseToolkit } from "server/types"; + +/** + * Prehandler that parses the payload and finds Buffers (i.e. files). + */ +export function getFiles(request: HapiRequest, _h: HapiResponseToolkit) { + const { uploadService } = request.services([]); + const files = uploadService.fileStreamsFromPayload(request.payload); + if (files.length) { + request.server.logger.info( + { id: request.yar.id, path: request.path }, + `Found ${uploadService.fileSummary(files)} to process on ${request.path}` + ); + return files; + } + + return null; +} diff --git a/runner/src/server/plugins/engine/pluginHandlers/files/prehandlers/handleUpload.ts b/runner/src/server/plugins/engine/pluginHandlers/files/prehandlers/handleUpload.ts new file mode 100644 index 0000000000..a775ff098c --- /dev/null +++ b/runner/src/server/plugins/engine/pluginHandlers/files/prehandlers/handleUpload.ts @@ -0,0 +1,119 @@ +import { HapiRequest, HapiResponseToolkit } from "server/types"; + +const parsedError = (key: string, error?: string) => { + return { + path: key, + href: `#${key}`, + name: key, + text: error, + }; +}; + +export async function handleUpload( + request: HapiRequest, + h: HapiResponseToolkit +) { + const { cacheService, uploadService } = request.services([]); + const state = await cacheService.getState(request); + let originalFilenames = state?.originalFilenames ?? {}; + let logger = request.server.logger; + const files = request.pre.files; + + if (!files) { + return h.continue; + } + + /** + * files is an array of tuples containing key and value. + * value may be an array of file data where multiple files have been uploaded + */ + const validFiles = request.pre.validFiles; + + for (const entry of validFiles) { + const [fieldName, streams] = entry; + const loggerIdentifier = uploadService.getLoggerIdentifier(request, { + fieldName, + }); + const previousUpload = originalFilenames[fieldName]; + + if (previousUpload) { + logger.info( + loggerIdentifier, + `User is attempting to overwrite ${fieldName} with location ${previousUpload.location}` + ); + } + + let response; + + try { + response = await uploadService.uploadDocuments(streams); + } catch (err) { + if (err.data?.res) { + const { error } = uploadService.parsedDocumentUploadResponse(err.data); + request.pre.errors = [ + ...request.pre.errors, + parsedError(fieldName, error), + ]; + } else if (err.code === "EPIPE") { + // ignore this error, it happens when the request is responded to by the doc upload service before the + // body has finished being sent. A valid response is still received. + logger.warn(loggerIdentifier, `Ignoring EPIPE response ${err.message}`); + } else { + logger.error( + { ...loggerIdentifier, err }, + `Error uploading document: ${err.message}` + ); + request.pre.errors = [ + ...(h.request.pre.errors || []), + parsedError(fieldName, err), + ]; + } + } + + const { location, warning, error } = response; + + if (location) { + const originalFilename = streams + .map((stream) => stream.hapi?.filename) + .join(", "); + + logger.info( + loggerIdentifier, + `Uploaded ${fieldName} successfully to ${location}` + ); + + originalFilenames[fieldName] = { location, originalFilename }; + const { + originalFilenames: updatedFilenames, + } = await cacheService.mergeState(request, { originalFilenames }); + + logger.info( + { ...loggerIdentifier, allFiles: updatedFilenames }, + `Updated originalFileNames for user` + ); + request.payload[fieldName] = location; + originalFilenames = updatedFilenames; + } + + if (warning) { + request.pre.warning = warning; + logger.warn( + loggerIdentifier, + `File was uploaded successfully but there was a warning ${warning}` + ); + } + + if (error) { + logger.error( + loggerIdentifier, + `Document upload API responded with an error ${error}` + ); + request.pre.errors = [ + ...(request.pre.errors || []), + parsedError(fieldName, error), + ]; + } + } + + return h.continue; +} diff --git a/runner/src/server/plugins/engine/pluginHandlers/files/prehandlers/index.ts b/runner/src/server/plugins/engine/pluginHandlers/files/prehandlers/index.ts new file mode 100644 index 0000000000..7404041602 --- /dev/null +++ b/runner/src/server/plugins/engine/pluginHandlers/files/prehandlers/index.ts @@ -0,0 +1,3 @@ +export { getFiles } from "./getFiles"; +export { validateContentTypes } from "./validateContentTypes"; +export { handleUpload } from "./handleUpload"; diff --git a/runner/src/server/plugins/engine/pluginHandlers/files/prehandlers/validateContentTypes.ts b/runner/src/server/plugins/engine/pluginHandlers/files/prehandlers/validateContentTypes.ts new file mode 100644 index 0000000000..533e7cbedf --- /dev/null +++ b/runner/src/server/plugins/engine/pluginHandlers/files/prehandlers/validateContentTypes.ts @@ -0,0 +1,96 @@ +import { HapiRequest, HapiResponseToolkit } from "server/types"; +import { ReadableStreamEntry } from "server/services/upload/uploadService"; + +/** + * This prehandler must follow {@link getFiles}. This prehandler checks if the content-type in the FormData is correct + * (or empty). If the user uploaded an empty field, a previous upload will be assigned if it exits. + */ +export async function validateContentTypes( + request: HapiRequest, + h: HapiResponseToolkit +) { + const files = request.pre.files; + + if (!files) { + return h.continue; + } + + const { uploadService, cacheService } = request.services([]); + const logger = request.server.logger; + const loggerIdentifier = { id: request.yar.id, path: request.path }; + + const validFields: ReadableStreamEntry[] = []; + const erroredFields: string[] = []; + + const { originalFilenames = {} } = await cacheService.getState(request); + + const { id, path } = request.params; + const form = request.server.app.forms[id]; + const page = form.pages.find((page) => page.path === `/${path}`); + const components = page.components.formItems; + + for (const [fieldName, values] of files) { + const component = components.find( + (component) => component.name === fieldName + ); + + const originalFilenameLocation = originalFilenames[fieldName]?.location; + + const filesArePopulated = values.every((value) => value?._data?.length > 1); + const componentIsOptional = component?.options?.required === false; + + if (!filesArePopulated && componentIsOptional) { + logger.warn( + loggerIdentifier, + `${fieldName} is optional, user skipped uploading a file. ${ + originalFilenameLocation + ? `Using ${originalFilenameLocation} instead` + : "" + }` + ); + + request.payload[fieldName] = originalFilenameLocation; + continue; + } + + const invalidFile = values.find( + (value) => !uploadService.validateContentType(value) + ); + + if (invalidFile) { + logger.error( + loggerIdentifier, + `User uploaded file with invalid content type or empty field for ${fieldName}, attempting to find previous upload` + ); + + if (!originalFilenameLocation) { + logger.error( + loggerIdentifier, + `User uploaded invalid content type or empty field for ${fieldName}, and has no previous upload for field. Deleting ${fieldName} from payload` + ); + delete request.payload[fieldName]; + erroredFields.push(fieldName); + } + + if (originalFilenameLocation) { + logger.warn( + loggerIdentifier, + `User uploaded invalid content type or empty field for ${fieldName}, using existing upload ${originalFilenameLocation} instead` + ); + request.payload[fieldName] = originalFilenameLocation; + } + + continue; + } + + validFields.push([fieldName, values]); + } + + if (erroredFields) { + request.pre.errors = erroredFields.map((field) => + uploadService.invalidFileTypeError(field) + ); + } + + return validFields; +} diff --git a/runner/src/server/plugins/engine/views/components/fileuploadfield.html b/runner/src/server/plugins/engine/views/components/fileuploadfield.html index fab533da45..085a0a2e86 100644 --- a/runner/src/server/plugins/engine/views/components/fileuploadfield.html +++ b/runner/src/server/plugins/engine/views/components/fileuploadfield.html @@ -2,7 +2,7 @@ {% macro FileUploadField(component) %} {{ govukFileUpload(component.model) }} {% if component.model.value %} -

You have already provided this file.

+

You previously uploaded the file ‘{{component.model.value}}’

{% endif %}