From e98701a97a506dfbc21b936de2d20453dbd9e3f7 Mon Sep 17 00:00:00 2001 From: Alex Luckett Date: Thu, 15 Jan 2026 15:06:39 +0000 Subject: [PATCH 1/2] only clear the nested page property out of state, not all file uploads --- src/server/forms/simple-form.yaml | 14 +++++++++ .../engine/pageControllers/errors.test.ts | 7 +++-- .../plugins/engine/pageControllers/errors.ts | 6 +++- src/server/services/cacheService.ts | 29 ++++++++++++++++--- 4 files changed, 49 insertions(+), 7 deletions(-) diff --git a/src/server/forms/simple-form.yaml b/src/server/forms/simple-form.yaml index 2f72d1fe4..efa8f8215 100644 --- a/src/server/forms/simple-form.yaml +++ b/src/server/forms/simple-form.yaml @@ -41,6 +41,20 @@ pages: schema: {} id: 987c1234-56d7-89e0-1234-56789abcdef0 id: 23456789-0abc-def1-2345-67890abcdef1 + - title: Upload a copy of your drivers licence + controller: FileUploadPageController + path: '/upload-driving-licence' + components: + - type: FileUploadField + title: Please upload a copy of your drivers licence + name: driversLicenceUpload + shortDescription: Upload drivers licence + hint: '' + options: + required: true + schema: {} + id: 987c1234-56d7-89e0-1234-56789abcdef1 + id: 23456789-0abc-def1-2345-67890abcdef2 - title: '' path: '/date-of-birth' components: diff --git a/src/server/plugins/engine/pageControllers/errors.test.ts b/src/server/plugins/engine/pageControllers/errors.test.ts index ac7c3910b..f74709180 100644 --- a/src/server/plugins/engine/pageControllers/errors.test.ts +++ b/src/server/plugins/engine/pageControllers/errors.test.ts @@ -16,7 +16,7 @@ describe('InvalidComponentStateError', () => { }) describe('getStateKeys', () => { - it('should return component name and upload for FileUploadField', () => { + it('should return component name and nested upload path for FileUploadField', () => { const page = model.pages.find((p) => p.path === '/file-upload-component') const component = new FileUploadField( { @@ -35,7 +35,10 @@ describe('InvalidComponentStateError', () => { ) const stateKeys = error.getStateKeys() - expect(stateKeys).toEqual(['fileUpload', 'upload']) + expect(stateKeys).toEqual([ + 'fileUpload', + "upload['/file-upload-component']" + ]) }) it('should return only component name for non-FileUploadField components', () => { diff --git a/src/server/plugins/engine/pageControllers/errors.ts b/src/server/plugins/engine/pageControllers/errors.ts index b0361b186..d3d2bf5c1 100644 --- a/src/server/plugins/engine/pageControllers/errors.ts +++ b/src/server/plugins/engine/pageControllers/errors.ts @@ -23,7 +23,11 @@ export class InvalidComponentStateError extends Error { getStateKeys() { const extraStateKeys = - this.component instanceof FileUploadField ? ['upload'] : [] + this.component instanceof FileUploadField + ? this.component.page?.path + ? [`upload['${this.component.page.path}']`] + : ['upload'] + : [] return [this.component.name].concat(extraStateKeys) } diff --git a/src/server/services/cacheService.ts b/src/server/services/cacheService.ts index 5a7be807d..3db9b9e18 100644 --- a/src/server/services/cacheService.ts +++ b/src/server/services/cacheService.ts @@ -1,5 +1,6 @@ import { type Server } from '@hapi/hapi' import * as Hoek from '@hapi/hoek' +import unset from 'lodash/unset.js' import { config } from '~/src/config/index.js' import { type createServer } from '~/src/server/index.js' @@ -99,6 +100,29 @@ export class CacheService { request.yar.flash(key.id, message) } + /** + * Resets (removes) component states from the form state by their keys. + * Supports both flat keys and nested paths. + * @param request - The Hapi request object + * @param componentNames - Array of state keys to remove. Uses lodash's unset syntax. Can be: + * - Flat keys: `'componentName'` for top-level state + * - Nested paths: `"upload['/my-page']"` or `'upload./my-page'` for nested state + * @example + * ```typescript + * // Remove a flat component state + * await cacheService.resetComponentStates(request, ['emailAddress']) + * + * // Remove nested upload state for a specific page + * await cacheService.resetComponentStates(request, ["upload['/file-upload-page']"]) + * + * // Remove multiple states at once + * await cacheService.resetComponentStates(request, [ + * 'componentName', + * "upload['/my-page']" + * ]) + * ``` + * @returns The updated state after removal + */ async resetComponentStates( request: AnyFormRequest, componentNames: string[] @@ -106,10 +130,7 @@ export class CacheService { const state = await this.getState(request) for (const componentName of componentNames) { - if (componentName in state) { - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete state[componentName] - } + unset(state, componentName) } return this.setState(request, state) From 759b70cb96a900c797b596e0a7a6b0dfc18baae0 Mon Sep 17 00:00:00 2001 From: Alex Luckett Date: Thu, 15 Jan 2026 15:52:27 +0000 Subject: [PATCH 2/2] Allow pages to declare their own supplementary state keys. Used when cleaning up state after failures. --- .../FileUploadPageController.test.ts | 50 ++++++++++++++++++- .../FileUploadPageController.ts | 22 +++++++- .../pageControllers/PageController.test.ts | 7 +++ .../engine/pageControllers/PageController.ts | 17 +++++++ .../plugins/engine/pageControllers/errors.ts | 7 +-- 5 files changed, 94 insertions(+), 9 deletions(-) diff --git a/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts b/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts index 14f82d0bb..32315eb91 100644 --- a/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts +++ b/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts @@ -2,7 +2,11 @@ import { ComponentType, type ComponentDef } from '@defra/forms-model' import { type ValidationErrorItem, type ValidationResult } from 'joi' -import { tempItemSchema } from '~/src/server/plugins/engine/components/FileUploadField.js' +import { + FileUploadField, + tempItemSchema +} from '~/src/server/plugins/engine/components/FileUploadField.js' +import { TextField } from '~/src/server/plugins/engine/components/TextField.js' import { getCacheService, getError @@ -1133,4 +1137,48 @@ describe('FileUploadPageController', () => { expect(controller.shouldShowSaveAndExit(serverWithSaveAndExit)).toBe(true) }) }) + + describe('getStateKeys', () => { + it('should return nested upload path for FileUploadField component', () => { + const component = controller.fileUpload + const stateKeys = controller.getStateKeys(component) + + expect(stateKeys).toEqual(["upload['/file-upload-component']"]) + }) + + it('should return empty array for non-FileUploadField components', () => { + const component = new TextField( + { + name: 'testField', + title: 'Test field', + type: ComponentType.TextField, + options: {}, + schema: {} + }, + { model, page: controller } + ) + + const stateKeys = controller.getStateKeys(component) + expect(stateKeys).toEqual([]) + }) + + it('should return fallback upload key when component has no page', () => { + const componentDef: ComponentDef = { + name: 'fileUpload', + title: 'Upload something', + type: ComponentType.FileUploadField, + options: {}, + schema: {} + } + + // Create a component without a page reference - should return ['upload'] + const component = new FileUploadField(componentDef, { + model, + page: undefined + }) + + const stateKeys = controller.getStateKeys(component) + expect(stateKeys).toEqual(['upload']) + }) + }) }) diff --git a/src/server/plugins/engine/pageControllers/FileUploadPageController.ts b/src/server/plugins/engine/pageControllers/FileUploadPageController.ts index 928ba86fc..79e36ca6e 100644 --- a/src/server/plugins/engine/pageControllers/FileUploadPageController.ts +++ b/src/server/plugins/engine/pageControllers/FileUploadPageController.ts @@ -4,9 +4,10 @@ import { wait } from '@hapi/hoek' import { type ValidationErrorItem } from 'joi' import { - tempItemSchema, - type FileUploadField + FileUploadField, + tempItemSchema } from '~/src/server/plugins/engine/components/FileUploadField.js' +import { type FormComponent } from '~/src/server/plugins/engine/components/FormComponent.js' import { getCacheService, getError, @@ -97,6 +98,23 @@ export class FileUploadPageController extends QuestionPageController { this.viewName = 'file-upload' } + /** + * Get supplementary state keys for clearing file upload state. + * Returns the nested upload path for FileUploadField components only. + * @param component - The component to get supplementary state keys for + * @returns Array containing the nested upload path, e.g., ["upload['/page-path']"] + * or ['upload'] if no page path is available. Returns empty array for non-FileUploadField components. + */ + getStateKeys(component: FormComponent): string[] { + // Only return upload keys for FileUploadField components + if (!(component instanceof FileUploadField)) { + return [] + } + + const pagePath = component.page?.path + return pagePath ? [`upload['${pagePath}']`] : ['upload'] + } + getFormDataFromState( request: FormContextRequest | undefined, state: FormSubmissionState diff --git a/src/server/plugins/engine/pageControllers/PageController.test.ts b/src/server/plugins/engine/pageControllers/PageController.test.ts index 73c7cb15d..48093dc44 100644 --- a/src/server/plugins/engine/pageControllers/PageController.test.ts +++ b/src/server/plugins/engine/pageControllers/PageController.test.ts @@ -236,4 +236,11 @@ describe('PageController', () => { ) }) }) + + describe('getStateKeys', () => { + it('should return empty array by default', () => { + const stateKeys = controller1.getStateKeys() + expect(stateKeys).toEqual([]) + }) + }) }) diff --git a/src/server/plugins/engine/pageControllers/PageController.ts b/src/server/plugins/engine/pageControllers/PageController.ts index 3254ff312..1cdf48f29 100644 --- a/src/server/plugins/engine/pageControllers/PageController.ts +++ b/src/server/plugins/engine/pageControllers/PageController.ts @@ -9,6 +9,7 @@ import Boom from '@hapi/boom' import { type Lifecycle, type RouteOptions, type Server } from '@hapi/hapi' import { type ComponentCollection } from '~/src/server/plugins/engine/components/ComponentCollection.js' +import { type FormComponent } from '~/src/server/plugins/engine/components/FormComponent.js' import { getSaveAndExitHelpers, getStartPath, @@ -175,6 +176,22 @@ export class PageController { throw Boom.badRequest('Unsupported POST route handler for this page') } + /** + * Get supplementary state keys for clearing component state. + * + * This method returns page controller-level state keys only. The core component's + * state key (the component's name) is managed separately by the framework and should + * NOT be included in the returned array. + * + * Returns an empty array by default. Override in subclasses to provide + * page-specific supplementary state keys (e.g., upload state, cached data). + * @param _component - The component to get supplementary state keys for (optional) + * @returns Array of supplementary state keys to clear (excluding the component name itself) + */ + getStateKeys(_component?: FormComponent): string[] { + return [] + } + shouldShowSaveAndExit(server: Server): boolean { return getSaveAndExitHelpers(server) !== undefined && this.allowSaveAndExit } diff --git a/src/server/plugins/engine/pageControllers/errors.ts b/src/server/plugins/engine/pageControllers/errors.ts index d3d2bf5c1..c96fb76f7 100644 --- a/src/server/plugins/engine/pageControllers/errors.ts +++ b/src/server/plugins/engine/pageControllers/errors.ts @@ -1,4 +1,3 @@ -import { FileUploadField } from '~/src/server/plugins/engine/components/FileUploadField.js' import { type FormComponent } from '~/src/server/plugins/engine/components/FormComponent.js' /** @@ -23,11 +22,7 @@ export class InvalidComponentStateError extends Error { getStateKeys() { const extraStateKeys = - this.component instanceof FileUploadField - ? this.component.page?.path - ? [`upload['${this.component.page.path}']`] - : ['upload'] - : [] + this.component.page?.getStateKeys(this.component) ?? [] return [this.component.name].concat(extraStateKeys) }