diff --git a/docs/GETTING_STARTED.md b/docs/GETTING_STARTED.md index bb4953329..4e428874e 100644 --- a/docs/GETTING_STARTED.md +++ b/docs/GETTING_STARTED.md @@ -111,7 +111,7 @@ const paths = [join(config.get('appDir'), 'views')] await server.register({ plugin, options: { - cacheName: 'session', // must match a session you've instantiated in your hapi server config + cache: 'session', // must match a session you've instantiated in your hapi server config. Also accepts a CacheService instance for advanced use-cases. /** * Options that DXT uses to render Nunjucks templates */ diff --git a/docs/PLUGIN_OPTIONS.md b/docs/PLUGIN_OPTIONS.md index 1208586c2..4a5421ff7 100644 --- a/docs/PLUGIN_OPTIONS.md +++ b/docs/PLUGIN_OPTIONS.md @@ -16,10 +16,13 @@ The forms plugin is configured with [registration options](https://hapi.dev/api/ - `controllers` (optional) - Object map of custom page controllers used to override the default. See [custom controllers](#custom-controllers) - `globals` (optional) - A map of custom template globals to include - `filters` (optional) - A map of custom template filters to include -- `cacheName` (optional) - The cache name to use. Defaults to hapi's [default server cache]. Recommended for production. See [here](#custom-cache) for more details +- `cache` (optional) - Caching options + - `cache` (optional) - Caching options. Recommended for production. This can be either: + - a string representing the cache name to use (e.g. hapi's default server cache). See [here](#custom-cache) for more details. + - a custom `CacheService` instance implementing your own caching logic - `pluginPath` (optional) - The location of the plugin (defaults to `node_modules/@defra/forms-engine-plugin`) - `preparePageEventRequestOptions` (optional) - A function that will be invoked for http-based [page events](./features/configuration-based/PAGE_EVENTS.md). See [here](./features/configuration-based/PAGE_EVENTS.md#authenticating-a-http-page-event-request-from-dxt-in-your-api) for details -- `saveAndReturn` (optional) - Configuration for custom session management including key generation, session hydration, and persistence. See [save and return documentation](./features/code-based/SAVE_AND_RETURN.md) for details +- `saveAndExit` (optional) - Configuration for custom session management including key generation, session hydration, and persistence. See [save and exit documentation](./features/code-based/SAVE_AND_EXIT.md) for details - `onRequest` (optional) - A function that will be invoked on each request to any form route e.g `/{slug}/{path}`. See [here](#onrequest) for more details ## Services @@ -80,7 +83,7 @@ If provided, the `onRequest` plugin option will be invoked on each request to an ```ts export type OnRequestCallback = ( - request: FormRequest | FormRequestPayload, + request: AnyFormRequest, params: FormParams, definition: FormDefinition, metadata: FormMetadata @@ -104,37 +107,35 @@ await server.register({ }) ``` -## saveAndReturn +## saveAndExit -The `saveAndReturn` plugin option enables custom session handling to enable "Save and Return" functionality. It consists of three optional functions: +The `saveAndExit` plugin option enables custom session handling to enable "Save and Exit" functionality. It is an optional route handler function that is called with the hapi request and response toolkit in addition to the last argument which is the [form context](./REQUEST_LIFECYCLE.md) of the current page from which the save and exit button was pressed: -- `keyGenerator` - Generates unique cache keys for session storage -- `sessionHydrator` - Retrieves saved session data from external sources -- `sessionPersister` - Stores session data to external systems +```ts +export type SaveAndExitHandler = ( + request: FormRequestPayload, + h: FormResponseToolkit, + context: FormContext +) => ResponseObject +``` ```js await server.register({ plugin, options: { - saveAndReturn: { - keyGenerator: (request) => { - const { userId, applicationId } = fetchSubmissionAttributes(request) - return `${userId}:${applicationId}` - }, - - sessionHydrator: async (request) => { - // Fetch saved state from database/API - const savedState = await fetchUserSession(request) - return savedState || null - }, - - sessionPersister: async (state, request) => { - // Save state to database/API - await saveUserSession(state, request) - } + saveAndExit: ( + request: FormRequestPayload, + h: FormResponseToolkit, + context: FormContext + ) => { + const { params } = request + const { slug } = params + + // Redirect user to custom page to handle saving + return h.redirect(`/custom-magic-link-save-and-exit/${slug}`) } } }) ``` -For detailed documentation and examples, see [Save and Return](./features/code-based/SAVE_AND_RETURN.md). +For detailed documentation and examples, see [Save and Exit](./features/code-based/SAVE_AND_EXIT.md). diff --git a/docs/features/code-based/SAVE_AND_EXIT.md b/docs/features/code-based/SAVE_AND_EXIT.md new file mode 100644 index 000000000..d613308f1 --- /dev/null +++ b/docs/features/code-based/SAVE_AND_EXIT.md @@ -0,0 +1,88 @@ +--- +layout: default +title: Save and exit +parent: Code-based Features +grand_parent: Features +render_with_liquid: false +--- + +# Save and Exit + +The forms engine supports save and exit capabilities through the `saveAndExit` plugin option. This feature enables applications to support end users saving their current answers and returning to the form at a later date. + +It does this by displaying a secondary button on each question page when the feature is enabled. When the button is clicked the form is submitted in the usual way and once the page data is validated, the provided `saveAndExit` handler is called. This is a standard hapi route handler with an additional `FormContext` parameter passed that contains the [current state of the users progression through the form](../../REQUEST_LIFECYCLE.md). + +> **Note:** it is your responsibility to ensure any state that exists outside of the form engine is captured upon persistence and available during hydration, e.g. file uploads via CDP. + +## Configuration + +The `saveAndExit` option is configured when registering the forms engine plugin: + +```ts +await server.register({ + plugin, + options: { + // ... other options + saveAndExit: ( + request: FormRequestPayload, + h: FormResponseToolkit, + context: FormContext + ): ResponseObject => {} + } +}) +``` + +It is down to you to provide the mechanism by which you want to store the users data and provide them a means by which they can return to it at a later data. The `saveAndExit` handler simply activates the additional button, gives you the hook point in to the framework and provides you the data you need to know where the user had progressed to. + +One common approach is ask end users for their email and send them a "magic link" that they can use to return with 28 days. + +``` +// This example shows how you can support custom UI flows to allow an end user to save their form progress and return at a later date. +// The save and exit method is called like other hapi route handlers and expects a similar return value. +// Here we're redirecting the user to another page where we might be providing a magic link or similar that the user can use to return to the form with. +await server.register({ + plugin, + options: { + saveAndExit: ( + request: FormRequestPayload, + h: FormResponseToolkit, + context: FormContext + ) => { + const { params } = request + const { slug } = params + const usersAnswers = context.state + + // Redirect user to custom page to handle saving + return h.redirect(`/custom-magic-link-save-and-exit/${slug}`) + } + } +}) +``` + +## Data Structure + +The `FormSubmissionState` object can be found at `context.state` and contains all the answers the user has provided so far. + +This is the data you'll need to save to allow users to pick up from where they left. + +```typescript +interface FormSubmissionState { + // User's form field values + [fieldName: string]: FormStateValue + + // Special system fields + $$__referenceNumber?: string + + // File upload state (if applicable) + upload?: Record +} +``` + +## Restore session data + +To restore a user's previous state use the `cacheService.setState` method. +The current request is passed in order to generate the cache key as so should include the correct form `slug` and `status` (if using the draft/live feature) + +```js +await cacheService.setState(request, state) +``` diff --git a/docs/features/code-based/SAVE_AND_RETURN.md b/docs/features/code-based/SAVE_AND_RETURN.md deleted file mode 100644 index fc73a3a50..000000000 --- a/docs/features/code-based/SAVE_AND_RETURN.md +++ /dev/null @@ -1,162 +0,0 @@ ---- -layout: default -title: Save and return -parent: Code-based Features -grand_parent: Features -render_with_liquid: false ---- - -# Save and Return - -The forms engine supports save and return capabilities through the `saveAndReturn` plugin option. This feature enables advanced session handling for applications that need custom session storage, retrieval, and management beyond the default in-memory Redis cache. - -## Overview - -- **Generate custom cache keys** for session storage, e.g. if you want to cache by user ID -- **Hydrate sessions** from external data sources (e.g. pre-filling a form when making a return journey) -- **Persist session data** to external systems for long-term storage (e.g. Saving data to return later) - -Using the above, users can save their progress and continue filling out forms later, even across different devices or browser sessions. - -> **Note:** it is your responsibility to ensure any state that exists outside of the form engine is captured upon persistence and available during hydration, e.g. file uploads via CDP. - -## Configuration - -The `saveAndReturn` option is configured when registering the forms engine plugin: - -```js -await server.register({ - plugin: formsEnginePlugin, - options: { - // ... other options - saveAndReturn: { - keyGenerator: (request) => string, - sessionHydrator: (request) => Promise, - sessionPersister: (state, request) => Promise - } - } -}) -``` - -## Functions - -### keyGenerator - -**Type:** `(request: RequestType) => string` - -Generates a cache key used to store and retrieve user session state. - -```js -const keyGenerator = (request) => { - const { userId, businessId, grantId } = request.app.userContext - return `${userId}:${businessId}:${grantId}` -} -``` - -**Parameters:** - -- `request` - The Hapi request object containing user context and form parameters - -**Returns:** A string that uniquely identifies the user's session - -### sessionHydrator - -**Type:** `(request: RequestType) => Promise` - -Called when no session state is found in Redis cache. This function should fetch saved state from an external source (e.g., database, API) and return it in the same structure expected by the form engine. This will generally be the same value as provided as `state` to the `sessionPersister` function, so a user can resume their session. - -```js -const sessionHydrator = async (request) => { - const { userId, businessId, grantId } = request.app.userContext - const key = `${userId}:${businessId}:${grantId}` - - try { - const response = await fetch(`https://backend.api/state/${key}`) - if (!response.ok) return null - - const state = await response.json() - return state // Must match FormSubmissionState structure - } catch (error) { - request.logger.error('Failed to hydrate session', error) - return null - } -} -``` - -**Parameters:** - -- `request` - The Hapi request object - -**Returns:** Promise that resolves to either: - -- `FormSubmissionState` object containing the user's saved form data -- `null` if no saved state is found or an error occurs - -### sessionPersister - -**Type:** `(state: FormSubmissionState, request: RequestType) => Promise` - -Called to persist session state to an external system for long-term storage. - -```js -const sessionPersister = async (state, request) => { - const { userId, businessId, grantId } = request.app.userContext - const key = `${userId}:${businessId}:${grantId}` - try { - await fetch(`https://your-backend.api/state/${key}`, { - method: 'PUT', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(state) - }) - - request.logger.info(`Session persisted for key: ${key}`) - } catch (error) { - request.logger.error('Failed to persist session', error) - throw error - } -} -``` - -**Parameters:** - -- `state` - The current form submission state to be persisted -- `request` - The Hapi request object - -**Returns:** Promise that resolves when the state is successfully persisted - -## Session Flow - -The session management system works as follows: - -1. **Key Generation**: When a user accesses a form, `keyGenerator` creates a unique cache key -2. **Cache Check**: The engine checks the cache for existing session data -3. **Hydration**: If no data exists in the cache, `sessionHydrator` is called to fetch from external storage -4. **Restoration**: Retrieved data is loaded back into Redis for fast access during the session -5. **Persistence**: When users save their progress, `sessionPersister` stores data to external storage - -Notes: - -- The rehydrated state must include enough information to satisfy schema validation on the current or next page. -- To properly resume a session, users should be redirected to the `/summary` page. The form engine will detect if the session state is incomplete, then the user will be redirected back to the last valid page. - -## Data Structure - -The `FormSubmissionState` object passed to and from session management functions contains: - -```typescript -interface FormSubmissionState { - // User's form field values - [fieldName: string]: FormStateValue - - // Special system fields - $$__referenceNumber?: string - - // File upload state (if applicable) - upload?: Record -} -``` - -## Error Handling - -- `sessionHydrator` should return `null` if no saved state is found or if errors occur -- `sessionPersister` should throw errors if persistence fails diff --git a/package.json b/package.json index c088fa0bb..ab12825bc 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,9 @@ "./services/*": "./.server/server/plugins/engine/services/*", "./engine/*": "./.server/server/plugins/engine/*", "./helpers.js": "./.server/server/plugins/engine/components/helpers.js", + "./schema.js": "./.server/server/schemas/index.js", "./templates/*": "./.server/server/plugins/engine/views/*", + "./cache-service.js": "./.server/server/services/cacheService.js", "./package.json": "./package.json" }, "scripts": { diff --git a/src/server/index.test.ts b/src/server/index.test.ts index 4d8808f30..79c1888c3 100644 --- a/src/server/index.test.ts +++ b/src/server/index.test.ts @@ -639,42 +639,3 @@ describe('prepareEnvironment', () => { ) }) }) - -describe('Exit route handlers', () => { - let server: Server - - beforeAll(async () => { - server = await createServer({ - services: defaultServices - }) - await server.initialize() - }) - - afterAll(async () => { - await server.stop() - }) - - beforeEach(() => { - jest.mocked(getFormMetadata).mockResolvedValue(fixtures.form.metadata) - server.app.models.clear() - }) - - test('GET /exit returns 200 with exit page content', async () => { - jest.mocked(getFormMetadata).mockResolvedValueOnce({ - ...fixtures.form.metadata, - live: fixtures.form.state - }) - - jest.mocked(getFormDefinition).mockResolvedValue(fixtures.form.definition) - - const options = { - method: 'GET', - url: `${FORM_PREFIX}/slug/exit` - } - - const res = await server.inject(options) - - expect(res.statusCode).toBe(StatusCodes.OK) - expect(res.result).toContain('Your progress has been saved') - }) -}) diff --git a/src/server/index.ts b/src/server/index.ts index 9af3e79ad..f4a3aa7f4 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -82,8 +82,11 @@ export async function createServer(routeConfig?: RouteConfig) { prepareSecureContext(server) } + const cacheService = routeConfig?.cacheServiceCreator + ? routeConfig.cacheServiceCreator(server) + : undefined const pluginCrumb = configureCrumbPlugin(routeConfig) - const pluginEngine = await configureEnginePlugin(routeConfig) + const pluginEngine = await configureEnginePlugin(routeConfig, cacheService) await server.register(pluginSession) await server.register(pluginPulse) diff --git a/src/server/plugins/engine/README.md b/src/server/plugins/engine/README.md index 547f37c07..d21b9c630 100644 --- a/src/server/plugins/engine/README.md +++ b/src/server/plugins/engine/README.md @@ -86,9 +86,9 @@ There are a number of `LiquidJS` filters available to you from within the templa ] ``` -### Save and return +### Save and exit -See [our save and return feature page](/docs/features/code-based/SAVE_AND_RETURN.md). +See [our save and exit feature page](/docs/features/code-based/SAVE_AND_EXIT.md). ### Additional notes diff --git a/src/server/plugins/engine/components/helpers/helpers.test.ts b/src/server/plugins/engine/components/helpers/helpers.test.ts index a753bfabf..20fc10927 100644 --- a/src/server/plugins/engine/components/helpers/helpers.test.ts +++ b/src/server/plugins/engine/components/helpers/helpers.test.ts @@ -25,7 +25,7 @@ describe('helpers tests', () => { }) describe('ComponentBase tests', () => { - test('should handle save and return functionality', () => { + test('should handle save and exit functionality', () => { const mockComponentDef = { type: 'TextField', name: 'testField', diff --git a/src/server/plugins/engine/configureEnginePlugin.ts b/src/server/plugins/engine/configureEnginePlugin.ts index c003d48c2..dba5fa230 100644 --- a/src/server/plugins/engine/configureEnginePlugin.ts +++ b/src/server/plugins/engine/configureEnginePlugin.ts @@ -10,17 +10,21 @@ import { formsService } from '~/src/server/plugins/engine/services/localFormsSer import { type PluginOptions } from '~/src/server/plugins/engine/types.js' import { findPackageRoot } from '~/src/server/plugins/engine/vision.js' import { devtoolContext } from '~/src/server/plugins/nunjucks/context.js' +import { type CacheService } from '~/src/server/services/cacheService.js' import { type RouteConfig } from '~/src/server/types.js' -export const configureEnginePlugin = async ({ - formFileName, - formFilePath, - services, - controllers, - preparePageEventRequestOptions, - onRequest, - saveAndReturn -}: RouteConfig = {}): Promise<{ +export const configureEnginePlugin = async ( + { + formFileName, + formFilePath, + services, + controllers, + preparePageEventRequestOptions, + onRequest, + saveAndExit + }: RouteConfig = {}, + cache?: CacheService +): Promise<{ plugin: typeof plugin options: PluginOptions }> => { @@ -50,7 +54,7 @@ export const configureEnginePlugin = async ({ formsService: await formsService() }, controllers, - cacheName: 'session', + cache: cache ?? 'session', nunjucks: { baseLayoutPath: 'dxt-devtool-baselayout.html', paths: [join(findPackageRoot(), 'src/server/devserver')] // custom layout to make it really clear this is not the same as the runner @@ -59,7 +63,7 @@ export const configureEnginePlugin = async ({ preparePageEventRequestOptions, onRequest, baseUrl: 'http://localhost:3009', // always runs locally - saveAndReturn + saveAndExit } } } diff --git a/src/server/plugins/engine/helpers.test.ts b/src/server/plugins/engine/helpers.test.ts index 632a431ca..323d645aa 100644 --- a/src/server/plugins/engine/helpers.test.ts +++ b/src/server/plugins/engine/helpers.test.ts @@ -30,7 +30,8 @@ import { import { FormAction, FormStatus, - type FormRequest + type FormRequest, + type FormResponseToolkit } from '~/src/server/routes/types.js' import definition from '~/test/form/definitions/basic.js' import templateDefinition from '~/test/form/definitions/templates.js' @@ -47,7 +48,7 @@ type HrefFilter = (this: NunjucksContext, path: string) => string | undefined describe('Helpers', () => { let page: PageControllerClass let request: FormContextRequest - let h: Pick + let h: FormResponseToolkit beforeEach(() => { const model = new FormModel(definition, { diff --git a/src/server/plugins/engine/helpers.ts b/src/server/plugins/engine/helpers.ts index 9f6ffed16..a3c2b2dac 100644 --- a/src/server/plugins/engine/helpers.ts +++ b/src/server/plugins/engine/helpers.ts @@ -23,6 +23,7 @@ import { import { type FormModel } from '~/src/server/plugins/engine/models/FormModel.js' import { type PageControllerClass } from '~/src/server/plugins/engine/pageControllers/helpers/pages.js' import { + type AnyFormRequest, type FormContext, type FormContextRequest, type FormSubmissionError @@ -32,8 +33,7 @@ import { FormStatus, type FormParams, type FormQuery, - type FormRequest, - type FormRequestPayload + type FormResponseToolkit } from '~/src/server/routes/types.js' const logger = createLogger() @@ -117,7 +117,7 @@ engine.registerFilter('answer', function (name: string) { export function proceed( request: Pick, - h: Pick, + h: FormResponseToolkit, nextUrl: string ) { const { method, payload, query } = request @@ -327,7 +327,7 @@ export function getError(detail: ValidationErrorItem): FormSubmissionError { * is not disabled on the current route, and that cookies/state are present. */ export function safeGenerateCrumb( - request: FormRequest | FormRequestPayload | null + request: AnyFormRequest | null ): string | undefined { // no request or no .state if (!request?.state) { @@ -380,8 +380,8 @@ export function getCacheService(server: Server) { return getPluginOptions(server).cacheService } -export function getSaveAndReturnHelpers(server: Server) { - return getPluginOptions(server).saveAndReturn +export function getSaveAndExitHelpers(server: Server) { + return getPluginOptions(server).saveAndExit } export function getPluginOptions(server: Server) { diff --git a/src/server/plugins/engine/models/FormModel.test.ts b/src/server/plugins/engine/models/FormModel.test.ts index ddd0c8e45..8154c3c77 100644 --- a/src/server/plugins/engine/models/FormModel.test.ts +++ b/src/server/plugins/engine/models/FormModel.test.ts @@ -185,7 +185,7 @@ describe('FormModel', () => { }) describe('getFormContext', () => { - it.each([FormAction.Validate, FormAction.SaveAndReturn, undefined])( + it.each([FormAction.Validate, FormAction.SaveAndExit, undefined])( 'returns a form context with the correct payload and state when action is %s', (action) => { const formModel = new FormModel(fieldsRequiredDefinition, { @@ -219,7 +219,7 @@ describe('FormModel', () => { } ) - it('returns without updating the state when the action is not validate or saveAndReturn', () => { + it('returns without updating the state when the action is not validate or saveAndExit', () => { const formModel = new FormModel(fieldsRequiredDefinition, { basePath: '/components' }) diff --git a/src/server/plugins/engine/models/FormModel.ts b/src/server/plugins/engine/models/FormModel.ts index edf9576f0..21c838d91 100644 --- a/src/server/plugins/engine/models/FormModel.ts +++ b/src/server/plugins/engine/models/FormModel.ts @@ -543,8 +543,7 @@ function validateFormPayload( // Skip validation GET requests or other actions if ( !request.payload || - (action && - ![FormAction.Validate, FormAction.SaveAndReturn].includes(action)) + (action && ![FormAction.Validate, FormAction.SaveAndExit].includes(action)) ) { return context } diff --git a/src/server/plugins/engine/models/SummaryViewModel.test.ts b/src/server/plugins/engine/models/SummaryViewModel.test.ts index e5189dbd0..9c95c7165 100644 --- a/src/server/plugins/engine/models/SummaryViewModel.test.ts +++ b/src/server/plugins/engine/models/SummaryViewModel.test.ts @@ -5,7 +5,7 @@ import { } from '~/src/server/plugins/engine/models/index.js' import { SummaryPageController } from '~/src/server/plugins/engine/pageControllers/SummaryPageController.js' import { buildFormContextRequest } from '~/src/server/plugins/engine/pageControllers/__stubs__/request.js' -import { serverWithSaveAndReturn } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' +import { serverWithSaveAndExit } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' import { createPage, type PageControllerClass @@ -274,16 +274,16 @@ describe('SummaryPageController', () => { }, query: {}, app: { model }, - server: serverWithSaveAndReturn + server: serverWithSaveAndExit } }) - describe('Save and Return functionality', () => { - it('should show save and return button on summary page', () => { - expect(controller.shouldShowSaveAndReturn(request.server)).toBe(true) + describe('Save and Exit functionality', () => { + it('should show save and exit button on summary page', () => { + expect(controller.shouldShowSaveAndExit(request.server)).toBe(true) }) - it('should handle save and return from summary page', () => { + it('should handle save and exit from summary page', () => { const state: FormState = { $$__referenceNumber: 'foobar', orderType: 'collection', @@ -293,7 +293,7 @@ describe('SummaryPageController', () => { const context = model.getFormContext(request, state) const viewModel = controller.getViewModel(request, context) - expect(viewModel).toHaveProperty('allowSaveAndReturn', true) + expect(viewModel).toHaveProperty('allowSaveAndExit', true) }) it('should display correct page title', () => { diff --git a/src/server/plugins/engine/models/SummaryViewModel.ts b/src/server/plugins/engine/models/SummaryViewModel.ts index 1857b7fce..26308ab4f 100644 --- a/src/server/plugins/engine/models/SummaryViewModel.ts +++ b/src/server/plugins/engine/models/SummaryViewModel.ts @@ -51,7 +51,7 @@ export class SummaryViewModel { serviceUrl: string hasMissingNotificationEmail?: boolean components?: ComponentViewModel[] - allowSaveAndReturn = false + allowSaveAndExit = false constructor( request: FormContextRequest, diff --git a/src/server/plugins/engine/options.js b/src/server/plugins/engine/options.js index aec627846..39f04daca 100644 --- a/src/server/plugins/engine/options.js +++ b/src/server/plugins/engine/options.js @@ -1,6 +1,7 @@ import Joi from 'joi' import { createLogger } from '~/src/server/common/helpers/logging/logger.js' +import { CacheService } from '~/src/server/services/index.js' const logger = createLogger() @@ -8,7 +9,10 @@ const pluginRegistrationOptionsSchema = Joi.object({ model: Joi.object().optional(), services: Joi.object().optional(), controllers: Joi.object().pattern(Joi.string(), Joi.any()).optional(), - cacheName: Joi.string().optional(), + cache: Joi.alternatives().try( + Joi.object().instance(CacheService), + Joi.string() + ), globals: Joi.object().pattern(Joi.string(), Joi.any()).optional(), filters: Joi.object().pattern(Joi.string(), Joi.any()).optional(), pluginPath: Joi.string().optional(), @@ -20,11 +24,7 @@ const pluginRegistrationOptionsSchema = Joi.object({ preparePageEventRequestOptions: Joi.function().optional(), onRequest: Joi.function().optional(), baseUrl: Joi.string().uri().required(), - saveAndReturn: Joi.object({ - keyGenerator: Joi.function(), - sessionHydrator: Joi.function(), - sessionPersister: Joi.function() - }).optional() + saveAndExit: Joi.function().optional() }) /** diff --git a/src/server/plugins/engine/options.test.js b/src/server/plugins/engine/options.test.js index a13e77b35..8d4153d6e 100644 --- a/src/server/plugins/engine/options.test.js +++ b/src/server/plugins/engine/options.test.js @@ -19,7 +19,7 @@ describe('validatePluginOptions', () => { expect(validatePluginOptions(validOptions)).toEqual(validOptions) }) - it('accepts optional properties keyGenerator, sessionHydrator, and sessionPersister', () => { + it('accepts optional property saveAndExit', () => { /** * @type {PluginOptions} */ @@ -32,11 +32,7 @@ describe('validatePluginOptions', () => { return { hello: 'world' } }, baseUrl: 'http://localhost:3009', - saveAndReturn: { - keyGenerator: () => 'test-key', - sessionHydrator: () => Promise.resolve({ someState: 'value' }), - sessionPersister: () => Promise.resolve(undefined) - } + saveAndExit: (request, h) => h.redirect('/save-and-exit') } expect(validatePluginOptions(validOptionsWithOptionals)).toEqual( diff --git a/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts b/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts index b7249f584..1f5c5bcbe 100644 --- a/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts +++ b/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts @@ -1,6 +1,5 @@ /* eslint-disable @typescript-eslint/dot-notation */ import { ComponentType, type ComponentDef } from '@defra/forms-model' -import { type ResponseToolkit } from '@hapi/hapi' import { type ValidationErrorItem, type ValidationResult } from 'joi' import { tempItemSchema } from '~/src/server/plugins/engine/components/FileUploadField.js' @@ -14,7 +13,7 @@ import { prepareStatus } from '~/src/server/plugins/engine/pageControllers/FileUploadPageController.js' import { QuestionPageController } from '~/src/server/plugins/engine/pageControllers/QuestionPageController.js' -import { serverWithSaveAndReturn } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' +import { serverWithSaveAndExit } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' import * as pageHelpers from '~/src/server/plugins/engine/pageControllers/helpers/index.js' import * as uploadService from '~/src/server/plugins/engine/services/uploadService.js' import { @@ -30,7 +29,8 @@ import { } from '~/src/server/plugins/engine/types.js' import { type FormRequest, - type FormRequestPayload + type FormRequestPayload, + type FormResponseToolkit } from '~/src/server/routes/types.js' import { type CacheService } from '~/src/server/services/index.js' import definition from '~/test/form/definitions/file-upload-basic.js' @@ -1040,7 +1040,7 @@ describe('FileUploadPageController', () => { } as unknown as FormRequest const context = { state } as unknown as FormContext - const h = {} as unknown as Pick + const h = {} as unknown as FormResponseToolkit const handler = controller.makeGetItemDeleteRouteHandler() @@ -1058,7 +1058,7 @@ describe('FileUploadPageController', () => { const h = { redirect: jest.fn() - } as unknown as Pick + } as unknown as FormResponseToolkit const context = { state: {} @@ -1119,11 +1119,9 @@ describe('FileUploadPageController', () => { }) }) - describe('shouldShowSaveAndReturn', () => { - it('should return true when save and return is enabled', () => { - expect(controller.shouldShowSaveAndReturn(serverWithSaveAndReturn)).toBe( - true - ) + describe('shouldShowSaveAndExit', () => { + it('should return true when save and exit is enabled', () => { + expect(controller.shouldShowSaveAndExit(serverWithSaveAndExit)).toBe(true) }) }) }) diff --git a/src/server/plugins/engine/pageControllers/FileUploadPageController.ts b/src/server/plugins/engine/pageControllers/FileUploadPageController.ts index d361ccf14..523cdfe50 100644 --- a/src/server/plugins/engine/pageControllers/FileUploadPageController.ts +++ b/src/server/plugins/engine/pageControllers/FileUploadPageController.ts @@ -1,6 +1,5 @@ import { ComponentType, type PageFileUpload } from '@defra/forms-model' import Boom from '@hapi/boom' -import { type ResponseToolkit } from '@hapi/hapi' import { wait } from '@hapi/hoek' import { type ValidationErrorItem } from 'joi' @@ -23,6 +22,7 @@ import { import { FileStatus, UploadStatus, + type AnyFormRequest, type FeaturedFormPageViewModel, type FileState, type FormContext, @@ -35,7 +35,8 @@ import { } from '~/src/server/plugins/engine/types.js' import { type FormRequest, - type FormRequestPayload + type FormRequestPayload, + type FormResponseToolkit } from '~/src/server/routes/types.js' const MAX_UPLOADS = 25 @@ -111,7 +112,7 @@ export class FileUploadPageController extends QuestionPageController { return payload } - async getState(request: FormRequest | FormRequestPayload) { + async getState(request: AnyFormRequest) { const { fileUpload } = this // Get the actual state @@ -148,7 +149,7 @@ export class FileUploadPageController extends QuestionPageController { return ( request: FormRequest, context: FormContext, - h: Pick + h: FormResponseToolkit ) => { const { viewModel } = this const { params } = request @@ -183,7 +184,7 @@ export class FileUploadPageController extends QuestionPageController { return async ( request: FormRequestPayload, context: FormContext, - h: Pick + h: FormResponseToolkit ) => { const { path } = this const { state } = context @@ -279,7 +280,7 @@ export class FileUploadPageController extends QuestionPageController { * @param state - the form state */ private async refreshUpload( - request: FormRequest | FormRequestPayload, + request: AnyFormRequest, state: FormSubmissionState ) { state = await this.checkUploadStatus(request, state) @@ -295,7 +296,7 @@ export class FileUploadPageController extends QuestionPageController { * @param depth - the number of retries so far */ private async checkUploadStatus( - request: FormRequest | FormRequestPayload, + request: AnyFormRequest, state: FormSubmissionState, depth = 1 ): Promise { @@ -417,7 +418,7 @@ export class FileUploadPageController extends QuestionPageController { * @param state - the form state */ private async initiateAndStoreNewUpload( - request: FormRequest | FormRequestPayload, + request: AnyFormRequest, state: FormSubmissionState ) { const { fileUpload, href, path } = this diff --git a/src/server/plugins/engine/pageControllers/PageController.test.ts b/src/server/plugins/engine/pageControllers/PageController.test.ts index d71056c24..8b30b4e98 100644 --- a/src/server/plugins/engine/pageControllers/PageController.test.ts +++ b/src/server/plugins/engine/pageControllers/PageController.test.ts @@ -3,8 +3,11 @@ import { type ResponseToolkit } from '@hapi/hapi' import { FORM_PREFIX } from '~/src/server/constants.js' import { FormModel } from '~/src/server/plugins/engine/models/FormModel.js' import { PageController } from '~/src/server/plugins/engine/pageControllers/PageController.js' -import { serverWithSaveAndReturn } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' -import { type FormRequest } from '~/src/server/routes/types.js' +import { serverWithSaveAndExit } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' +import { + type FormRequest, + type FormResponseToolkit +} from '~/src/server/routes/types.js' import definition from '~/test/form/definitions/basic.js' describe('PageController', () => { @@ -155,7 +158,7 @@ describe('PageController', () => { app: { model } } as FormRequest - const h: Pick = { + const h: FormResponseToolkit = { redirect: jest.fn(), view: jest.fn() } @@ -206,10 +209,10 @@ describe('PageController', () => { ) }) - it('supports save and return functionality', async () => { + it('supports save and exit functionality', async () => { const mockRequest = { ...request, - payload: { saveAndReturn: true } + payload: { saveAndExit: true } } as FormRequest const mockResponse = { @@ -232,9 +235,9 @@ describe('PageController', () => { }) }) - describe('shouldShowSaveAndReturn', () => { - it('should return false (PageController does not allow save and return)', () => { - expect(controller1.shouldShowSaveAndReturn(serverWithSaveAndReturn)).toBe( + describe('shouldShowSaveAndExit', () => { + it('should return false (PageController does not allow save and exit)', () => { + expect(controller1.shouldShowSaveAndExit(serverWithSaveAndExit)).toBe( false ) }) diff --git a/src/server/plugins/engine/pageControllers/PageController.ts b/src/server/plugins/engine/pageControllers/PageController.ts index d3c475998..0ba533749 100644 --- a/src/server/plugins/engine/pageControllers/PageController.ts +++ b/src/server/plugins/engine/pageControllers/PageController.ts @@ -6,17 +6,12 @@ import { type Section } from '@defra/forms-model' import Boom from '@hapi/boom' -import { - type Lifecycle, - type ResponseToolkit, - type RouteOptions, - type Server -} from '@hapi/hapi' +import { type Lifecycle, type RouteOptions, type Server } from '@hapi/hapi' import { type ComponentCollection } from '~/src/server/plugins/engine/components/ComponentCollection.js' import { encodeUrl, - getSaveAndReturnHelpers, + getSaveAndExitHelpers, getStartPath, normalisePath } from '~/src/server/plugins/engine/helpers.js' @@ -30,7 +25,8 @@ import { type FormRequest, type FormRequestPayload, type FormRequestPayloadRefs, - type FormRequestRefs + type FormRequestRefs, + type FormResponseToolkit } from '~/src/server/routes/types.js' export class PageController { @@ -47,7 +43,7 @@ export class PageController { events?: Events collection?: ComponentCollection viewName = 'index' - allowSaveAndReturn = false + allowSaveAndExit = false constructor(model: FormModel, pageDef: Page) { const { def } = model @@ -171,7 +167,7 @@ export class PageController { makeGetRouteHandler(): ( request: FormRequest, context: FormContext, - h: Pick + h: FormResponseToolkit ) => ReturnType> { return (request, context, h) => { const { viewModel, viewName } = this @@ -182,14 +178,12 @@ export class PageController { makePostRouteHandler(): ( request: FormRequestPayload, context: FormContext, - h: Pick + h: FormResponseToolkit ) => ReturnType> { throw Boom.badRequest('Unsupported POST route handler for this page') } - shouldShowSaveAndReturn(server: Server): boolean { - return ( - getSaveAndReturnHelpers(server) !== undefined && this.allowSaveAndReturn - ) + shouldShowSaveAndExit(server: Server): boolean { + return getSaveAndExitHelpers(server) !== undefined && this.allowSaveAndExit } } diff --git a/src/server/plugins/engine/pageControllers/QuestionPageController.test.ts b/src/server/plugins/engine/pageControllers/QuestionPageController.test.ts index 7555c6bcc..172e2df1b 100644 --- a/src/server/plugins/engine/pageControllers/QuestionPageController.test.ts +++ b/src/server/plugins/engine/pageControllers/QuestionPageController.test.ts @@ -1,14 +1,12 @@ import { type PageQuestion } from '@defra/forms-model' -import { type ResponseToolkit } from '@hapi/hapi' -import { getCacheService } from '~/src/server/plugins/engine/helpers.js' import { FormModel } from '~/src/server/plugins/engine/models/FormModel.js' import { QuestionPageController } from '~/src/server/plugins/engine/pageControllers/QuestionPageController.js' import { buildFormContextRequest, buildFormRequest } from '~/src/server/plugins/engine/pageControllers/__stubs__/request.js' -import { serverWithSaveAndReturn } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' +import { serverWithSaveAndExit } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' import { type FormContext, type FormPageViewModel, @@ -17,7 +15,8 @@ import { } from '~/src/server/plugins/engine/types.js' import { type FormRequest, - type FormRequestPayload + type FormRequestPayload, + type FormResponseToolkit } from '~/src/server/routes/types.js' import { CacheService } from '~/src/server/services/cacheService.js' import conditionalReveal from '~/test/form/definitions/conditional-reveal.js' @@ -594,7 +593,7 @@ describe('QuestionPageController', () => { code: jest.fn().mockImplementation(() => response) } - const h: Pick = { + const h: FormResponseToolkit = { redirect: jest.fn().mockReturnValue(response), view: jest.fn() } @@ -1155,7 +1154,7 @@ describe('QuestionPageController V2', () => { code: jest.fn().mockImplementation(() => response) } - const h: Pick = { + const h: FormResponseToolkit = { redirect: jest.fn().mockReturnValue(response), view: jest.fn() } @@ -1283,7 +1282,7 @@ describe('QuestionPageController V2', () => { }) }) -describe('Save and Return functionality', () => { +describe('Save and Exit functionality', () => { let model: FormModel let controller1: QuestionPageController let requestPage1: FormRequest @@ -1314,7 +1313,7 @@ describe('Save and Return functionality', () => { code: jest.fn().mockImplementation(() => response) } - const h: Pick = { + const h: FormResponseToolkit = { redirect: jest.fn().mockReturnValue(response), view: jest.fn() } @@ -1324,17 +1323,17 @@ describe('Save and Return functionality', () => { jest.spyOn(CacheService.prototype, 'setState') }) - describe('shouldShowSaveAndReturn', () => { + describe('shouldShowSaveAndExit', () => { it('should return true by default', () => { - expect(controller1.shouldShowSaveAndReturn(serverWithSaveAndReturn)).toBe( + expect(controller1.shouldShowSaveAndExit(serverWithSaveAndExit)).toBe( true ) }) }) - describe('handleSaveAndReturn', () => { - it('should save state and redirect to exit page', async () => { - const sessionPersisterMock = jest.fn() + describe('handleSaveAndExit', () => { + it('should invoke saveAndExit plugin option', () => { + const saveAndExitMock = jest.fn(() => ({})) const state: FormSubmissionState = { $$__referenceNumber: 'foobar', yesNoField: true @@ -1344,9 +1343,7 @@ describe('Save and Return functionality', () => { server: { plugins: { 'forms-engine-plugin': { - saveAndReturn: { - sessionPersister: sessionPersisterMock - }, + saveAndExit: saveAndExitMock, cacheService: { clearState: jest.fn() } as unknown as CacheService @@ -1354,52 +1351,18 @@ describe('Save and Return functionality', () => { } }, method: 'post', - payload: { yesNoField: true, action: 'save-and-return' } - } as unknown as FormRequestPayload - - const cacheService = getCacheService(request.server) - - const context = model.getFormContext(request, state) - - await controller1.handleSaveAndReturn(request, context, h) - - expect(sessionPersisterMock).toHaveBeenCalledWith(context.state, request) - expect(cacheService.clearState).toHaveBeenCalledWith(request) - expect(h.redirect).toHaveBeenCalledWith('/test/exit') - }) - - it('should throw if sessionPersister inside saveAndReturn options provided', async () => { - const sessionPersisterMock = jest.fn() - const state: FormSubmissionState = { - $$__referenceNumber: 'foobar', - yesNoField: true - } - const request = { - ...requestPage1, - server: { - plugins: { - 'forms-engine-plugin': { - // No sessionPersister object - saveAndReturn: {} - } - } - }, - method: 'post', - payload: { yesNoField: true, action: 'save-and-return' } + payload: { yesNoField: true, action: 'save-and-exit' } } as unknown as FormRequestPayload const context = model.getFormContext(request, state) - await expect( - controller1.handleSaveAndReturn(request, context, h) - ).rejects.toThrow('Server misconfigured for save and return') + controller1.handleSaveAndExit(request, context, h) - expect(sessionPersisterMock).not.toHaveBeenCalled() - expect(h.redirect).not.toHaveBeenCalled() + expect(saveAndExitMock).toHaveBeenCalledWith(request, h, context) }) - it('should throw if no saveAndReturn options provided', async () => { - const sessionPersisterMock = jest.fn() + it('should throw if saveAndExit option not provided', () => { + const saveAndExitMock = jest.fn() const state: FormSubmissionState = { $$__referenceNumber: 'foobar', yesNoField: true @@ -1409,57 +1372,27 @@ describe('Save and Return functionality', () => { server: { plugins: { 'forms-engine-plugin': { - // No saveAndReturn object + // No function + saveAndExit: undefined } } }, method: 'post', - payload: { yesNoField: true, action: 'save-and-return' } - } as unknown as FormRequestPayload - - const context = model.getFormContext(request, state) - - await expect( - controller1.handleSaveAndReturn(request, context, h) - ).rejects.toThrow('Server misconfigured for save and return') - - expect(sessionPersisterMock).not.toHaveBeenCalled() - expect(h.redirect).not.toHaveBeenCalled() - }) - - it('should throw if sessionPersister throws as well with validation errors', async () => { - const sessionPersisterMock = jest.fn().mockImplementation(() => { - throw new Error('Session persister error') - }) - const state: FormSubmissionState = { $$__referenceNumber: 'foobar' } - const request = { - ...requestPage1, - method: 'post', - server: { - plugins: { - 'forms-engine-plugin': { - saveAndReturn: { - sessionPersister: sessionPersisterMock - } - } - } - }, - payload: { action: 'save-and-return' } + payload: { yesNoField: true, action: 'save-and-exit' } } as unknown as FormRequestPayload const context = model.getFormContext(request, state) - await expect( - controller1.handleSaveAndReturn(request, context, h) - ).rejects.toThrow('Session persister error') + expect(() => controller1.handleSaveAndExit(request, context, h)).toThrow( + 'Server misconfigured for save and exit' + ) - expect(sessionPersisterMock).toHaveBeenCalledWith(context.state, request) - expect(h.redirect).not.toHaveBeenCalledWith('/test/exit') + expect(saveAndExitMock).not.toHaveBeenCalled() }) }) - describe('POST handler with save-and-return action', () => { - it('should handle FormAction.SaveAndReturn', async () => { + describe('POST handler with save-and-exit action', () => { + it('should handle FormAction.SaveAndExit', async () => { const state: FormSubmissionState = { $$__referenceNumber: 'foobar', yesNoField: true @@ -1467,27 +1400,27 @@ describe('Save and Return functionality', () => { const request = { ...requestPage1, method: 'post', - payload: { yesNoField: true, action: 'save-and-return' } + payload: { yesNoField: true, action: 'save-and-exit' } } as unknown as FormRequestPayload const context = model.getFormContext(request, state) jest.spyOn(controller1, 'getState').mockResolvedValue({}) jest - .spyOn(controller1, 'handleSaveAndReturn') - .mockResolvedValue(h.redirect('/test/exit')) + .spyOn(controller1, 'handleSaveAndExit') + .mockReturnValue(h.redirect('/custom-save-and-exit')) const postHandler = controller1.makePostRouteHandler() await postHandler(request, context, h) - expect(controller1.handleSaveAndReturn).toHaveBeenCalledWith( + expect(controller1.handleSaveAndExit).toHaveBeenCalledWith( request, context, h ) }) - it('should not call handleSaveAndReturn for continue action', async () => { + it('should not call handleSaveAndExit for continue action', async () => { const state: FormSubmissionState = { $$__referenceNumber: 'foobar', yesNoField: true @@ -1502,8 +1435,8 @@ describe('Save and Return functionality', () => { jest.spyOn(controller1, 'getState').mockResolvedValue({}) jest - .spyOn(controller1, 'handleSaveAndReturn') - .mockResolvedValue(h.redirect('/test/exit')) + .spyOn(controller1, 'handleSaveAndExit') + .mockReturnValue(h.redirect('/custom-save-and-exit')) jest.spyOn(controller1, 'setState').mockResolvedValue(state) const mockResponse = { @@ -1518,7 +1451,7 @@ describe('Save and Return functionality', () => { const postHandler = controller1.makePostRouteHandler() await postHandler(request, context, mockH) - expect(controller1.handleSaveAndReturn).not.toHaveBeenCalled() + expect(controller1.handleSaveAndExit).not.toHaveBeenCalled() }) }) }) diff --git a/src/server/plugins/engine/pageControllers/QuestionPageController.ts b/src/server/plugins/engine/pageControllers/QuestionPageController.ts index c43afb82f..478602d74 100644 --- a/src/server/plugins/engine/pageControllers/QuestionPageController.ts +++ b/src/server/plugins/engine/pageControllers/QuestionPageController.ts @@ -9,7 +9,7 @@ import { type Page } from '@defra/forms-model' import Boom from '@hapi/boom' -import { type ResponseToolkit, type RouteOptions } from '@hapi/hapi' +import { type RouteOptions } from '@hapi/hapi' import { type ValidationErrorItem } from 'joi' import { ComponentCollection } from '~/src/server/plugins/engine/components/ComponentCollection.js' @@ -18,13 +18,14 @@ import { type BackLink } from '~/src/server/plugins/engine/components/types.js' import { getCacheService, getErrors, - getSaveAndReturnHelpers, + getSaveAndExitHelpers, normalisePath, proceed } from '~/src/server/plugins/engine/helpers.js' import { type FormModel } from '~/src/server/plugins/engine/models/index.js' import { PageController } from '~/src/server/plugins/engine/pageControllers/PageController.js' import { + type AnyFormRequest, type FormContext, type FormContextRequest, type FormPageViewModel, @@ -39,7 +40,8 @@ import { type FormRequest, type FormRequestPayload, type FormRequestPayloadRefs, - type FormRequestRefs + type FormRequestRefs, + type FormResponseToolkit } from '~/src/server/routes/types.js' import { actionSchema, @@ -51,7 +53,7 @@ import { merge } from '~/src/server/services/cacheService.js' export class QuestionPageController extends PageController { collection: ComponentCollection errorSummaryTitle = 'There is a problem' - allowSaveAndReturn = true + allowSaveAndExit = true constructor(model: FormModel, pageDef: Page) { super(model, pageDef) @@ -177,14 +179,11 @@ export class QuestionPageController extends PageController { showTitle, components, errors, - allowSaveAndReturn: this.shouldShowSaveAndReturn(request.server) + allowSaveAndExit: this.shouldShowSaveAndExit(request.server) } } - getRelevantPath( - request: FormRequest | FormRequestPayload, - context: FormContext - ) { + getRelevantPath(request: AnyFormRequest, context: FormContext) { const { paths } = context const startPath = this.getStartPath() @@ -296,7 +295,7 @@ export class QuestionPageController extends PageController { return getErrors(details) } - async getState(request: FormRequest | FormRequestPayload) { + async getState(request: AnyFormRequest) { const { query } = request // Skip get for preview URL direct access @@ -309,10 +308,7 @@ export class QuestionPageController extends PageController { return cacheService.getState(request) } - async setState( - request: FormRequest | FormRequestPayload, - state: FormSubmissionState - ) { + async setState(request: AnyFormRequest, state: FormSubmissionState) { const { query } = request // Skip set for preview URL direct access @@ -326,7 +322,7 @@ export class QuestionPageController extends PageController { } async mergeState( - request: FormRequest | FormRequestPayload, + request: AnyFormRequest, state: FormSubmissionState, update: object ) { @@ -397,7 +393,7 @@ export class QuestionPageController extends PageController { return async ( request: FormRequest, context: FormContext, - h: Pick + h: FormResponseToolkit ) => { const { collection, model, viewName } = this const { evaluationState } = context @@ -492,7 +488,7 @@ export class QuestionPageController extends PageController { return async ( request: FormRequestPayload, context: FormContext, - h: Pick + h: FormResponseToolkit ) => { const { collection, viewName, model } = this const { isForceAccess, state, evaluationState } = context @@ -515,10 +511,10 @@ export class QuestionPageController extends PageController { return h.view(viewName, viewModel) } - // Check if this is a save-and-return action + // Check if this is a save-and-exit action const { action } = request.payload - if (action === FormAction.SaveAndReturn) { - return this.handleSaveAndReturn(request, context, h) + if (action === FormAction.SaveAndExit) { + return this.handleSaveAndExit(request, context, h) } // Save and proceed @@ -529,7 +525,7 @@ export class QuestionPageController extends PageController { proceed( request: FormContextRequest, - h: Pick, + h: FormResponseToolkit, nextPath?: string ) { const nextUrl = nextPath @@ -540,28 +536,20 @@ export class QuestionPageController extends PageController { } /** - * Handle save-and-return action by processing form data and redirecting to exit page + * Handle save-and-exit action */ - async handleSaveAndReturn( + handleSaveAndExit( request: FormRequestPayload, context: FormContext, - h: Pick + h: FormResponseToolkit ) { - const { state } = context + const saveAndExit = getSaveAndExitHelpers(request.server) - // Save the current state and redirect to exit page - const saveAndReturn = getSaveAndReturnHelpers(request.server) - - if (!saveAndReturn?.sessionPersister) { - throw Boom.internal('Server misconfigured for save and return') + if (!saveAndExit) { + throw Boom.internal('Server misconfigured for save and exit') } - await saveAndReturn.sessionPersister(state, request) - - const cacheService = getCacheService(request.server) - await cacheService.clearState(request) - - return h.redirect(this.getHref('/exit')) + return saveAndExit(request, h, context) } /** diff --git a/src/server/plugins/engine/pageControllers/RepeatPageController.test.ts b/src/server/plugins/engine/pageControllers/RepeatPageController.test.ts index f9b4b5c00..775a896ee 100644 --- a/src/server/plugins/engine/pageControllers/RepeatPageController.test.ts +++ b/src/server/plugins/engine/pageControllers/RepeatPageController.test.ts @@ -3,7 +3,7 @@ import { RepeatPageController } from '~/src/server/plugins/engine/pageController import { buildFormContextRequest } from '~/src/server/plugins/engine/pageControllers/__stubs__/request.js' import { server, - serverWithSaveAndReturn + serverWithSaveAndExit } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' import { type FormContextRequest, @@ -271,11 +271,9 @@ describe('RepeatPageController', () => { }) }) - describe('shouldShowSaveAndReturn', () => { - it('should return true when save and return is enabled', () => { - expect(controller.shouldShowSaveAndReturn(serverWithSaveAndReturn)).toBe( - true - ) + describe('shouldShowSaveAndExit', () => { + it('should return true when save and exit is enabled', () => { + expect(controller.shouldShowSaveAndExit(serverWithSaveAndExit)).toBe(true) }) }) }) diff --git a/src/server/plugins/engine/pageControllers/RepeatPageController.ts b/src/server/plugins/engine/pageControllers/RepeatPageController.ts index 5dbe86b58..3252b8fa2 100644 --- a/src/server/plugins/engine/pageControllers/RepeatPageController.ts +++ b/src/server/plugins/engine/pageControllers/RepeatPageController.ts @@ -2,7 +2,6 @@ import { randomUUID } from 'crypto' import { type PageRepeat, type Repeat } from '@defra/forms-model' import Boom from '@hapi/boom' -import { type ResponseToolkit } from '@hapi/hapi' import Joi from 'joi' import { isRepeatState } from '~/src/server/plugins/engine/components/FormComponent.js' @@ -25,7 +24,8 @@ import { import { FormAction, type FormRequest, - type FormRequestPayload + type FormRequestPayload, + type FormResponseToolkit } from '~/src/server/routes/types.js' export class RepeatPageController extends QuestionPageController { @@ -128,10 +128,7 @@ export class RepeatPageController extends QuestionPageController { } } - proceed( - request: FormContextRequest, - h: Pick - ) { + proceed(request: FormContextRequest, h: FormResponseToolkit) { const nextPath = this.getSummaryPath(request) return super.proceed(request, h, nextPath) } @@ -151,7 +148,7 @@ export class RepeatPageController extends QuestionPageController { return async ( request: FormRequest, context: FormContext, - h: Pick + h: FormResponseToolkit ) => { const { path } = this const { query } = request @@ -179,7 +176,7 @@ export class RepeatPageController extends QuestionPageController { return ( request: FormRequest, context: FormContext, - h: Pick + h: FormResponseToolkit ) => { const { path } = this const { query } = request @@ -205,7 +202,7 @@ export class RepeatPageController extends QuestionPageController { return ( request: FormRequestPayload, context: FormContext, - h: Pick + h: FormResponseToolkit ) => { const { path, repeat } = this const { query } = request @@ -269,7 +266,7 @@ export class RepeatPageController extends QuestionPageController { return ( request: FormRequest, context: FormContext, - h: Pick + h: FormResponseToolkit ) => { const { viewModel } = this const { state } = context @@ -304,7 +301,7 @@ export class RepeatPageController extends QuestionPageController { return async ( request: FormRequestPayload, context: FormContext, - h: Pick + h: FormResponseToolkit ) => { const { repeat } = this const { state } = context diff --git a/src/server/plugins/engine/pageControllers/StartPageController.test.ts b/src/server/plugins/engine/pageControllers/StartPageController.test.ts index a7324f7c5..c4411a442 100644 --- a/src/server/plugins/engine/pageControllers/StartPageController.test.ts +++ b/src/server/plugins/engine/pageControllers/StartPageController.test.ts @@ -1,6 +1,6 @@ import { FormModel } from '~/src/server/plugins/engine/models/FormModel.js' import { StartPageController } from '~/src/server/plugins/engine/pageControllers/StartPageController.js' -import { serverWithSaveAndReturn } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' +import { serverWithSaveAndExit } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' import definition from '~/test/form/definitions/basic.js' describe('StartPageController', () => { @@ -22,9 +22,9 @@ describe('StartPageController', () => { controller = new StartPageController(model, mockPage as any) }) - describe('shouldShowSaveAndReturn', () => { - it('should return false (StartPageController does not allow save and return)', () => { - expect(controller.shouldShowSaveAndReturn(serverWithSaveAndReturn)).toBe( + describe('shouldShowSaveAndExit', () => { + it('should return false (StartPageController does not allow save and exit)', () => { + expect(controller.shouldShowSaveAndExit(serverWithSaveAndExit)).toBe( false ) }) diff --git a/src/server/plugins/engine/pageControllers/StartPageController.ts b/src/server/plugins/engine/pageControllers/StartPageController.ts index 4cf59cf8a..db4403916 100644 --- a/src/server/plugins/engine/pageControllers/StartPageController.ts +++ b/src/server/plugins/engine/pageControllers/StartPageController.ts @@ -9,7 +9,7 @@ export class StartPageController extends QuestionPageController { * but start pages should really live on gov.uk (whitehall publisher) so a user can be properly signposted. */ - allowSaveAndReturn = false + allowSaveAndExit = false getViewModel(request: FormRequest, context: FormContext) { return { diff --git a/src/server/plugins/engine/pageControllers/StatusPageController.test.ts b/src/server/plugins/engine/pageControllers/StatusPageController.test.ts index 324a64b5e..f459c388d 100644 --- a/src/server/plugins/engine/pageControllers/StatusPageController.test.ts +++ b/src/server/plugins/engine/pageControllers/StatusPageController.test.ts @@ -1,6 +1,6 @@ import { FormModel } from '~/src/server/plugins/engine/models/FormModel.js' import { StatusPageController } from '~/src/server/plugins/engine/pageControllers/StatusPageController.js' -import { serverWithSaveAndReturn } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' +import { serverWithSaveAndExit } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' import definition from '~/test/form/definitions/basic.js' describe('StatusPageController', () => { @@ -22,9 +22,9 @@ describe('StatusPageController', () => { controller = new StatusPageController(model, mockPage as any) }) - describe('shouldShowSaveAndReturn', () => { - it('should return false (StatusPageController does not allow save and return)', () => { - expect(controller.shouldShowSaveAndReturn(serverWithSaveAndReturn)).toBe( + describe('shouldShowSaveAndExit', () => { + it('should return false (StatusPageController does not allow save and exit)', () => { + expect(controller.shouldShowSaveAndExit(serverWithSaveAndExit)).toBe( false ) }) diff --git a/src/server/plugins/engine/pageControllers/StatusPageController.ts b/src/server/plugins/engine/pageControllers/StatusPageController.ts index e5a1f5563..9f2f72d7e 100644 --- a/src/server/plugins/engine/pageControllers/StatusPageController.ts +++ b/src/server/plugins/engine/pageControllers/StatusPageController.ts @@ -1,15 +1,17 @@ import { type PageStatus } from '@defra/forms-model' -import { type ResponseToolkit } from '@hapi/hapi' import { getCacheService } from '~/src/server/plugins/engine/helpers.js' import { type FormModel } from '~/src/server/plugins/engine/models/index.js' import { QuestionPageController } from '~/src/server/plugins/engine/pageControllers/QuestionPageController.js' import { type FormContext } from '~/src/server/plugins/engine/types.js' -import { type FormRequest } from '~/src/server/routes/types.js' +import { + type FormRequest, + type FormResponseToolkit +} from '~/src/server/routes/types.js' export class StatusPageController extends QuestionPageController { declare pageDef: PageStatus - allowSaveAndReturn = false + allowSaveAndExit = false constructor(model: FormModel, pageDef: PageStatus) { super(model, pageDef) @@ -24,7 +26,7 @@ export class StatusPageController extends QuestionPageController { return async ( request: FormRequest, context: FormContext, - h: Pick + h: FormResponseToolkit ) => { const { viewModel, viewName } = this diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.ts index 23ad3ea00..14d4ecae4 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.ts @@ -5,7 +5,7 @@ import { type SubmitPayload } from '@defra/forms-model' import Boom from '@hapi/boom' -import { type ResponseToolkit, type RouteOptions } from '@hapi/hapi' +import { type RouteOptions } from '@hapi/hapi' import { ComponentCollection } from '~/src/server/plugins/engine/components/ComponentCollection.js' import { FileUploadField } from '~/src/server/plugins/engine/components/FileUploadField.js' @@ -30,13 +30,16 @@ import { type FormSubmissionState } from '~/src/server/plugins/engine/types.js' import { + FormAction, type FormRequest, type FormRequestPayload, - type FormRequestPayloadRefs + type FormRequestPayloadRefs, + type FormResponseToolkit } from '~/src/server/routes/types.js' export class SummaryPageController extends QuestionPageController { declare pageDef: Page + allowSaveAndExit = true /** * The controller which is used when Page["controller"] is defined as "./pages/summary.js" @@ -69,7 +72,7 @@ export class SummaryPageController extends QuestionPageController { viewModel.feedbackLink = this.feedbackLink viewModel.phaseTag = this.phaseTag viewModel.components = components - viewModel.allowSaveAndReturn = this.shouldShowSaveAndReturn(request.server) + viewModel.allowSaveAndExit = this.shouldShowSaveAndExit(request.server) return viewModel } @@ -81,7 +84,7 @@ export class SummaryPageController extends QuestionPageController { return async ( request: FormRequest, context: FormContext, - h: Pick + h: FormResponseToolkit ) => { const { viewName } = this @@ -102,10 +105,17 @@ export class SummaryPageController extends QuestionPageController { return async ( request: FormRequestPayload, context: FormContext, - h: Pick + h: FormResponseToolkit ) => { const { model } = this const { params } = request + + // Check if this is a save-and-exit action + const { action } = request.payload + if (action === FormAction.SaveAndExit) { + return this.handleSaveAndExit(request, context, h) + } + const cacheService = getCacheService(request.server) const { formsService } = this.model.services diff --git a/src/server/plugins/engine/pageControllers/TerminalController.test.ts b/src/server/plugins/engine/pageControllers/TerminalController.test.ts index 988df7900..d7829a05c 100644 --- a/src/server/plugins/engine/pageControllers/TerminalController.test.ts +++ b/src/server/plugins/engine/pageControllers/TerminalController.test.ts @@ -1,6 +1,6 @@ import { FormModel } from '~/src/server/plugins/engine/models/FormModel.js' import { TerminalPageController } from '~/src/server/plugins/engine/pageControllers/TerminalPageController.js' -import { serverWithSaveAndReturn } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' +import { serverWithSaveAndExit } from '~/src/server/plugins/engine/pageControllers/__stubs__/server.js' import definition from '~/test/form/definitions/basic.js' describe('TerminalController', () => { @@ -27,9 +27,9 @@ describe('TerminalController', () => { }) }) - describe('shouldShowSaveAndReturn', () => { - it('should return false (TerminalPageController does not allow save and return)', () => { - expect(controller1.shouldShowSaveAndReturn(serverWithSaveAndReturn)).toBe( + describe('shouldShowSaveAndExit', () => { + it('should return false (TerminalPageController does not allow save and exit)', () => { + expect(controller1.shouldShowSaveAndExit(serverWithSaveAndExit)).toBe( false ) }) diff --git a/src/server/plugins/engine/pageControllers/TerminalPageController.ts b/src/server/plugins/engine/pageControllers/TerminalPageController.ts index be5c87fef..a9e05cc9a 100644 --- a/src/server/plugins/engine/pageControllers/TerminalPageController.ts +++ b/src/server/plugins/engine/pageControllers/TerminalPageController.ts @@ -1,19 +1,22 @@ import { type PageTerminal } from '@defra/forms-model' import Boom from '@hapi/boom' -import { type ResponseObject, type ResponseToolkit } from '@hapi/hapi' +import { type ResponseObject } from '@hapi/hapi' import { QuestionPageController } from '~/src/server/plugins/engine/pageControllers/QuestionPageController.js' import { type FormContext } from '~/src/server/plugins/engine/types.js' -import { type FormRequestPayload } from '~/src/server/routes/types.js' +import { + type FormRequestPayload, + type FormResponseToolkit +} from '~/src/server/routes/types.js' export class TerminalPageController extends QuestionPageController { declare pageDef: PageTerminal - allowSaveAndReturn = false + allowSaveAndExit = false makePostRouteHandler(): ( request: FormRequestPayload, context: FormContext, - h: Pick + h: FormResponseToolkit ) => Promise { throw Boom.methodNotAllowed('POST method not allowed for terminal pages') } diff --git a/src/server/plugins/engine/pageControllers/__stubs__/server.ts b/src/server/plugins/engine/pageControllers/__stubs__/server.ts index 8fe8c6cfb..7dbf79b10 100644 --- a/src/server/plugins/engine/pageControllers/__stubs__/server.ts +++ b/src/server/plugins/engine/pageControllers/__stubs__/server.ts @@ -12,16 +12,15 @@ export const server: Server = { } } as Server // only mocking out properties we care about; -export const serverWithSaveAndReturn: Server = { +export const serverWithSaveAndExit: Server = { plugins: { ...server.plugins, 'forms-engine-plugin': { ...server.plugins['forms-engine-plugin'], - saveAndReturn: { - keyGenerator: jest.fn().mockReturnValue('foobar'), - sessionHydrator: jest.fn().mockReturnValue({}), - sessionPersister: jest.fn().mockImplementation(() => Promise.resolve()) - } as Pick + saveAndExit: jest.fn().mockReturnValue({}) as Pick< + PluginOptions, + 'saveAndExit' + > } } } as Server // only mocking out properties we care about diff --git a/src/server/plugins/engine/plugin.ts b/src/server/plugins/engine/plugin.ts index f915614c4..2c783d38d 100644 --- a/src/server/plugins/engine/plugin.ts +++ b/src/server/plugins/engine/plugin.ts @@ -8,7 +8,6 @@ import { import { type FormModel } from '~/src/server/plugins/engine/models/index.js' import { validatePluginOptions } from '~/src/server/plugins/engine/options.js' -import { getRoutes as getSaveAndReturnExitRoutes } from '~/src/server/plugins/engine/routes/exit.js' import { getRoutes as getFileUploadStatusRoutes } from '~/src/server/plugins/engine/routes/file-upload.js' import { makeLoadFormPreHandler } from '~/src/server/plugins/engine/routes/index.js' import { getRoutes as getQuestionRoutes } from '~/src/server/plugins/engine/routes/questions.js' @@ -31,28 +30,24 @@ export const plugin = { const { model, - cacheName, - saveAndReturn, + cache, + saveAndExit, nunjucks: nunjucksOptions, viewContext, preparePageEventRequestOptions } = options - const cacheService = new CacheService({ - server, - cacheName, - options: { - keyGenerator: saveAndReturn?.keyGenerator, - sessionHydrator: saveAndReturn?.sessionHydrator - } - }) + const cacheService = + typeof cache === 'string' + ? new CacheService({ server, cacheName: cache }) + : cache await registerVision(server, options) server.expose('baseLayoutPath', nunjucksOptions.baseLayoutPath) server.expose('viewContext', viewContext) server.expose('cacheService', cacheService) - server.expose('saveAndReturn', saveAndReturn) + server.expose('saveAndExit', saveAndExit) server.app.model = model @@ -92,7 +87,6 @@ export const plugin = { ), ...getRepeaterSummaryRoutes(getRouteOptions, postRouteOptions), ...getRepeaterItemDeleteRoutes(getRouteOptions, postRouteOptions), - ...getSaveAndReturnExitRoutes(getRouteOptions), ...getFileUploadStatusRoutes() ] diff --git a/src/server/plugins/engine/routes/exit.ts b/src/server/plugins/engine/routes/exit.ts deleted file mode 100644 index a7c4c01ee..000000000 --- a/src/server/plugins/engine/routes/exit.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { slugSchema } from '@defra/forms-model' -import Boom from '@hapi/boom' -import { type ResponseToolkit, type RouteOptions } from '@hapi/hapi' -import Joi from 'joi' - -import { - type FormRequest, - type FormRequestRefs -} from '~/src/server/routes/types.js' - -export function getRoutes(getRouteOptions: RouteOptions) { - return [ - { - method: 'get', - path: '/{slug}/exit', - handler: ( - request: FormRequest, - h: Pick - ) => { - const { app } = request - const { model } = app - - if (!model) { - throw Boom.notFound('No model found for exit page') - } - - const returnUrl = request.query.returnUrl - - const exitViewModel = { - pageTitle: 'Your progress has been saved', - phaseTag: model.def.phaseBanner?.phase, - returnUrl - } - - return h.view('exit', exitViewModel) - }, - options: { - ...getRouteOptions, - validate: { - params: Joi.object().keys({ - slug: slugSchema - }) - } - } - } - ] -} diff --git a/src/server/plugins/engine/routes/index.ts b/src/server/plugins/engine/routes/index.ts index dbefdd99d..c34b2ad53 100644 --- a/src/server/plugins/engine/routes/index.ts +++ b/src/server/plugins/engine/routes/index.ts @@ -21,17 +21,18 @@ import { type PageControllerClass } from '~/src/server/plugins/engine/pageContro import { generateUniqueReference } from '~/src/server/plugins/engine/referenceNumbers.js' import * as defaultServices from '~/src/server/plugins/engine/services/index.js' import { + type AnyFormRequest, type FormContext, type PluginOptions } from '~/src/server/plugins/engine/types.js' import { type FormRequest, - type FormRequestPayload + type FormResponseToolkit } from '~/src/server/routes/types.js' export async function redirectOrMakeHandler( - request: FormRequest | FormRequestPayload, - h: Pick, + request: AnyFormRequest, + h: FormResponseToolkit, makeHandler: ( page: PageControllerClass, context: FormContext @@ -92,10 +93,7 @@ export function makeLoadFormPreHandler(server: Server, options: PluginOptions) { const { formsService } = services - async function handler( - request: FormRequest | FormRequestPayload, - h: ResponseToolkit - ) { + async function handler(request: AnyFormRequest, h: ResponseToolkit) { if (server.app.model) { request.app.model = server.app.model @@ -181,10 +179,7 @@ export function makeLoadFormPreHandler(server: Server, options: PluginOptions) { return handler } -export function dispatchHandler( - request: FormRequest, - h: Pick -) { +export function dispatchHandler(request: FormRequest, h: FormResponseToolkit) { const { model } = request.app const servicePath = model ? `/${model.basePath}` : '' diff --git a/src/server/plugins/engine/routes/questions.test.ts b/src/server/plugins/engine/routes/questions.test.ts index baf8d3f6f..c9940ed5d 100644 --- a/src/server/plugins/engine/routes/questions.test.ts +++ b/src/server/plugins/engine/routes/questions.test.ts @@ -1,5 +1,5 @@ import Boom from '@hapi/boom' -import { type ResponseObject, type ResponseToolkit } from '@hapi/hapi' +import { type ResponseObject } from '@hapi/hapi' // eslint-disable-next-line n/no-unpublished-import import nock from 'nock' @@ -10,10 +10,14 @@ import { makeGetHandler, makePostHandler } from '~/src/server/plugins/engine/routes/questions.js' -import { type FormContext } from '~/src/server/plugins/engine/types.js' +import { + type AnyFormRequest, + type FormContext +} from '~/src/server/plugins/engine/types.js' import { type FormRequest, - type FormRequestPayload + type FormRequestPayload, + type FormResponseToolkit } from '~/src/server/routes/types.js' jest.mock('~/src/server/plugins/engine/models/SummaryViewModel', () => ({ SummaryViewModel: class { @@ -35,7 +39,7 @@ jest.mock('~/src/server/plugins/engine/outputFormatters/machine/v1', () => ({ jest.mock('~/src/server/plugins/engine/routes/index') describe('makeGetHandler', () => { - const hMock: Pick = { + const hMock: FormResponseToolkit = { redirect: jest.fn(), view: jest.fn() } @@ -64,7 +68,7 @@ describe('makeGetHandler', () => { ( _request: FormRequest, context: FormContext, - _h: Pick + _h: FormResponseToolkit ) => { data = context.data return Promise.resolve({} as unknown as ResponseObject) @@ -80,12 +84,8 @@ describe('makeGetHandler', () => { jest .mocked(redirectOrMakeHandler) - .mockImplementation( - ( - _req: FormRequest | FormRequestPayload, - _h: Pick, - fn - ) => Promise.resolve(fn(pageMock, contextMock)) + .mockImplementation((_req: AnyFormRequest, _h: FormResponseToolkit, fn) => + Promise.resolve(fn(pageMock, contextMock)) ) await makeGetHandler()(requestMock, hMock) @@ -108,7 +108,7 @@ describe('makeGetHandler', () => { ( _request: FormRequest, context: FormContext, - _h: Pick + _h: FormResponseToolkit ) => { data = context.data return Promise.resolve({} as unknown as ResponseObject) @@ -126,12 +126,8 @@ describe('makeGetHandler', () => { jest .mocked(redirectOrMakeHandler) - .mockImplementation( - ( - _req: FormRequest | FormRequestPayload, - _h: Pick, - fn - ) => Promise.resolve(fn(pageMock, contextMock)) + .mockImplementation((_req: AnyFormRequest, _h: FormResponseToolkit, fn) => + Promise.resolve(fn(pageMock, contextMock)) ) await makeGetHandler()(requestMock, hMock) @@ -152,7 +148,7 @@ describe('makeGetHandler', () => { ( _request: FormRequest, _context: FormContext, - _h: Pick + _h: FormResponseToolkit ) => { return Promise.resolve({} as unknown as ResponseObject) } @@ -168,11 +164,7 @@ describe('makeGetHandler', () => { jest .mocked(redirectOrMakeHandler) .mockImplementation( - async ( - _req: FormRequest | FormRequestPayload, - _h: Pick, - fn - ) => { + async (_req: AnyFormRequest, _h: FormResponseToolkit, fn) => { try { await fn(pageMock, contextMock) } catch (err) { @@ -190,7 +182,7 @@ describe('makeGetHandler', () => { }) describe('makePostHandler', () => { - const hMock: Pick = { + const hMock: FormResponseToolkit = { redirect: jest.fn(), view: jest.fn() } @@ -221,7 +213,7 @@ describe('makePostHandler', () => { ( _request: FormRequest, _context: FormContext, - _h: Pick + _h: FormResponseToolkit ) => { // do return a valid ResponseObject wrapped in Promise.resolve return mockPostResponse @@ -238,12 +230,8 @@ describe('makePostHandler', () => { jest .mocked(redirectOrMakeHandler) - .mockImplementation( - ( - _req: FormRequest | FormRequestPayload, - _h: Pick, - fn - ) => Promise.resolve(fn(pageMock, contextMock)) + .mockImplementation((_req: AnyFormRequest, _h: FormResponseToolkit, fn) => + Promise.resolve(fn(pageMock, contextMock)) ) const response = await makePostHandler()(requestMock, hMock) @@ -263,7 +251,7 @@ describe('makePostHandler', () => { ( _request: FormRequest, _context: FormContext, - _h: Pick + _h: FormResponseToolkit ) => { return Promise.resolve({} as unknown as ResponseObject) } @@ -281,12 +269,8 @@ describe('makePostHandler', () => { jest .mocked(redirectOrMakeHandler) - .mockImplementation( - ( - _req: FormRequest | FormRequestPayload, - _h: Pick, - fn - ) => Promise.resolve(fn(pageMock, contextMock)) + .mockImplementation((_req: AnyFormRequest, _h: FormResponseToolkit, fn) => + Promise.resolve(fn(pageMock, contextMock)) ) await makePostHandler()(requestMock, hMock) @@ -309,7 +293,7 @@ describe('makePostHandler', () => { ( _request: FormRequest, _context: FormContext, - _h: Pick + _h: FormResponseToolkit ) => { // do return a valid ResponseObject wrapped in Promise.resolve return mockPostResponse @@ -326,12 +310,8 @@ describe('makePostHandler', () => { jest .mocked(redirectOrMakeHandler) - .mockImplementation( - ( - _req: FormRequest | FormRequestPayload, - _h: Pick, - fn - ) => Promise.resolve(fn(pageMock, contextMock)) + .mockImplementation((_req: AnyFormRequest, _h: FormResponseToolkit, fn) => + Promise.resolve(fn(pageMock, contextMock)) ) await makePostHandler()(requestMock, hMock) @@ -352,7 +332,7 @@ describe('makePostHandler', () => { ( _request: FormRequest, _context: FormContext, - _h: Pick + _h: FormResponseToolkit ) => { return Promise.resolve({} as unknown as ResponseObject) } @@ -369,11 +349,7 @@ describe('makePostHandler', () => { jest .mocked(redirectOrMakeHandler) .mockImplementation( - async ( - _req: FormRequest | FormRequestPayload, - _h: Pick, - fn - ) => { + async (_req: AnyFormRequest, _h: FormResponseToolkit, fn) => { try { await fn(pageMock, contextMock) } catch (err) { @@ -395,7 +371,7 @@ function createMockPageController( routeHandler: ( request: FormRequest, context: FormContext, - h: Pick + h: FormResponseToolkit ) => ResponseObject | Promise ): PageControllerClass { return { diff --git a/src/server/plugins/engine/routes/questions.ts b/src/server/plugins/engine/routes/questions.ts index dc8d83dcd..bdc9a9659 100644 --- a/src/server/plugins/engine/routes/questions.ts +++ b/src/server/plugins/engine/routes/questions.ts @@ -2,7 +2,6 @@ import { hasFormComponents, slugSchema, type Event } from '@defra/forms-model' import Boom from '@hapi/boom' import { type ResponseObject, - type ResponseToolkit, type RouteOptions, type ServerRoute } from '@hapi/hapi' @@ -25,6 +24,7 @@ import { redirectOrMakeHandler } from '~/src/server/plugins/engine/routes/index.js' import { + type AnyFormRequest, type FormContext, type PreparePageEventRequestOptions } from '~/src/server/plugins/engine/types.js' @@ -32,7 +32,8 @@ import { type FormRequest, type FormRequestPayload, type FormRequestPayloadRefs, - type FormRequestRefs + type FormRequestRefs, + type FormResponseToolkit } from '~/src/server/routes/types.js' import { actionSchema, @@ -44,7 +45,7 @@ import { import * as httpService from '~/src/server/services/httpService.js' async function handleHttpEvent( - request: FormRequest | FormRequestPayload, + request: AnyFormRequest, page: PageControllerClass, context: FormContext, event: Event, @@ -75,10 +76,7 @@ async function handleHttpEvent( export function makeGetHandler( preparePageEventRequestOptions?: PreparePageEventRequestOptions ) { - return function getHandler( - request: FormRequest, - h: Pick - ) { + return function getHandler(request: FormRequest, h: FormResponseToolkit) { const { params } = request if (normalisePath(params.path) === '') { @@ -116,7 +114,7 @@ export function makePostHandler( ) { return function postHandler( request: FormRequestPayload, - h: Pick + h: FormResponseToolkit ) { const { query } = request diff --git a/src/server/plugins/engine/routes/repeaters/item-delete.ts b/src/server/plugins/engine/routes/repeaters/item-delete.ts index ad18c92ae..f1d06cd40 100644 --- a/src/server/plugins/engine/routes/repeaters/item-delete.ts +++ b/src/server/plugins/engine/routes/repeaters/item-delete.ts @@ -1,10 +1,6 @@ import { slugSchema } from '@defra/forms-model' import Boom from '@hapi/boom' -import { - type ResponseToolkit, - type RouteOptions, - type ServerRoute -} from '@hapi/hapi' +import { type RouteOptions, type ServerRoute } from '@hapi/hapi' import Joi from 'joi' import { FileUploadPageController } from '~/src/server/plugins/engine/pageControllers/FileUploadPageController.js' @@ -14,7 +10,8 @@ import { type FormRequest, type FormRequestPayload, type FormRequestPayloadRefs, - type FormRequestRefs + type FormRequestRefs, + type FormResponseToolkit } from '~/src/server/routes/types.js' import { actionSchema, @@ -26,10 +23,7 @@ import { } from '~/src/server/schemas/index.js' // Item delete GET route -function getHandler( - request: FormRequest, - h: Pick -) { +function getHandler(request: FormRequest, h: FormResponseToolkit) { const { params } = request return redirectOrMakeHandler(request, h, (page, context) => { @@ -46,10 +40,7 @@ function getHandler( }) } -function postHandler( - request: FormRequestPayload, - h: Pick -) { +function postHandler(request: FormRequestPayload, h: FormResponseToolkit) { const { params } = request return redirectOrMakeHandler(request, h, (page, context) => { diff --git a/src/server/plugins/engine/routes/repeaters/summary.ts b/src/server/plugins/engine/routes/repeaters/summary.ts index 557980dca..3126ed9d2 100644 --- a/src/server/plugins/engine/routes/repeaters/summary.ts +++ b/src/server/plugins/engine/routes/repeaters/summary.ts @@ -1,11 +1,7 @@ // List summary GET route import { slugSchema } from '@defra/forms-model' import Boom from '@hapi/boom' -import { - type ResponseToolkit, - type RouteOptions, - type ServerRoute -} from '@hapi/hapi' +import { type RouteOptions, type ServerRoute } from '@hapi/hapi' import Joi from 'joi' import { RepeatPageController } from '~/src/server/plugins/engine/pageControllers/RepeatPageController.js' @@ -14,7 +10,8 @@ import { type FormRequest, type FormRequestPayload, type FormRequestPayloadRefs, - type FormRequestRefs + type FormRequestRefs, + type FormResponseToolkit } from '~/src/server/routes/types.js' import { actionSchema, @@ -23,10 +20,7 @@ import { stateSchema } from '~/src/server/schemas/index.js' -function getHandler( - request: FormRequest, - h: Pick -) { +function getHandler(request: FormRequest, h: FormResponseToolkit) { const { params } = request return redirectOrMakeHandler(request, h, (page, context) => { @@ -38,10 +32,7 @@ function getHandler( }) } -function postHandler( - request: FormRequestPayload, - h: Pick -) { +function postHandler(request: FormRequestPayload, h: FormResponseToolkit) { const { params } = request return redirectOrMakeHandler(request, h, (page, context) => { diff --git a/src/server/plugins/engine/types.ts b/src/server/plugins/engine/types.ts index 91b98676a..7576545d7 100644 --- a/src/server/plugins/engine/types.ts +++ b/src/server/plugins/engine/types.ts @@ -7,7 +7,11 @@ import { type List, type Page } from '@defra/forms-model' -import { type PluginProperties, type Request } from '@hapi/hapi' +import { + type PluginProperties, + type Request, + type ResponseObject +} from '@hapi/hapi' import { type JoiExpression, type ValidationErrorItem } from 'joi' import { FormComponent } from '~/src/server/plugins/engine/components/FormComponent.js' @@ -36,12 +40,15 @@ import { type FormParams, type FormRequest, type FormRequestPayload, + type FormResponseToolkit, type FormStatus } from '~/src/server/routes/types.js' +import { type CacheService } from '~/src/server/services/cacheService.js' import { type RequestOptions } from '~/src/server/services/httpService.js' import { type Services } from '~/src/server/types.js' -type RequestType = Request | FormRequest | FormRequestPayload +export type AnyFormRequest = FormRequest | FormRequestPayload +export type AnyRequest = Request | AnyFormRequest /** * Form submission state stores the following in Redis: @@ -312,7 +319,7 @@ export interface FormPageViewModel extends PageViewModelBase { context: FormContext errors?: FormSubmissionError[] hasMissingNotificationEmail?: boolean - allowSaveAndReturn: boolean + allowSaveAndExit: boolean } export interface RepeaterSummaryPageViewModel extends PageViewModelBase { @@ -357,27 +364,26 @@ export type PreparePageEventRequestOptions = ( ) => void export type OnRequestCallback = ( - request: FormRequest | FormRequestPayload, + request: AnyFormRequest, params: FormParams, definition: FormDefinition, metadata: FormMetadata ) => void +export type SaveAndExitHandler = ( + request: FormRequestPayload, + h: FormResponseToolkit, + context: FormContext +) => ResponseObject + export interface PluginOptions { model?: FormModel services?: Services controllers?: Record - cacheName?: string + cache?: CacheService | string globals?: Record filters?: Record - saveAndReturn?: { - keyGenerator: (request: RequestType) => string - sessionHydrator: (request: RequestType) => Promise - sessionPersister: ( - state: FormSubmissionState, - request: RequestType - ) => Promise - } + saveAndExit?: SaveAndExitHandler pluginPath?: string nunjucks: { baseLayoutPath: string diff --git a/src/server/plugins/engine/types/index.ts b/src/server/plugins/engine/types/index.ts index e7e7e00f1..ac2ffe65a 100644 --- a/src/server/plugins/engine/types/index.ts +++ b/src/server/plugins/engine/types/index.ts @@ -1,4 +1,6 @@ export type { + AnyFormRequest, + AnyRequest, CheckAnswers, ErrorMessageTemplate, ErrorMessageTemplateList, @@ -74,7 +76,8 @@ export type { FormRequest, FormRequestPayload, FormRequestPayloadRefs, - FormRequestRefs + FormRequestRefs, + FormResponseToolkit } from '~/src/server/routes/types.js' export { FormAction, FormStatus } from '~/src/server/routes/types.js' diff --git a/src/server/plugins/engine/views/partials/form.html b/src/server/plugins/engine/views/partials/form.html index fb3476725..c7a68d32e 100644 --- a/src/server/plugins/engine/views/partials/form.html +++ b/src/server/plugins/engine/views/partials/form.html @@ -13,12 +13,12 @@ preventDoubleClick: true }) }} - {% if allowSaveAndReturn %} + {% if allowSaveAndExit %} {{ govukButton({ - text: "Save and return", + text: "Save and exit", classes: "govuk-button--secondary", name: "action", - value: "save-and-return", + value: "save-and-exit", preventDoubleClick: true }) }} {% endif %} diff --git a/src/server/plugins/engine/views/summary.html b/src/server/plugins/engine/views/summary.html index f6dfc4218..22a112dc9 100644 --- a/src/server/plugins/engine/views/summary.html +++ b/src/server/plugins/engine/views/summary.html @@ -2,6 +2,7 @@ {% from "govuk/components/error-summary/macro.njk" import govukErrorSummary %} {% from "govuk/components/summary-list/macro.njk" import govukSummaryList %} +{% from "govuk/components/button/macro.njk" import govukButton %} {% from "partials/components.html" import componentList with context %} {% block content %} @@ -31,7 +32,6 @@

- {% if declaration %}

Declaration

@@ -42,10 +42,26 @@

Declaration

{{ componentList(components) }} - {% set isDeclaration = declaration or components | length %} - +
+ {% set isDeclaration = declaration or components | length %} + + {{ govukButton({ + text: "Accept and send" if isDeclaration else "Send", + name: "action", + value: "send", + preventDoubleClick: true + }) }} + + {% if allowSaveAndExit %} + {{ govukButton({ + text: "Save and exit", + classes: "govuk-button--secondary", + name: "action", + value: "save-and-exit", + preventDoubleClick: true + }) }} + {% endif %} +
diff --git a/src/server/plugins/nunjucks/context.js b/src/server/plugins/nunjucks/context.js index 6d39a3e50..ef5b9dfca 100644 --- a/src/server/plugins/nunjucks/context.js +++ b/src/server/plugins/nunjucks/context.js @@ -18,7 +18,7 @@ const logger = createLogger() let webpackManifest /** - * @param {FormRequest | FormRequestPayload | null} request + * @param {AnyFormRequest | null} request */ export async function context(request) { const { params, response } = request ?? {} @@ -62,7 +62,7 @@ export async function context(request) { /** * Returns the context for the devtool. Consumers won't have access to this. - * @param {FormRequest | FormRequestPayload | null} _request + * @param {AnyFormRequest | null} _request * @returns {Record & { assetPath: string, getDxtAssetPath: (asset: string) => string }} */ export function devtoolContext(_request) { @@ -97,5 +97,5 @@ export function devtoolContext(_request) { /** * @import { ViewContext } from '~/src/server/plugins/nunjucks/types.js' - * @import { FormRequest, FormRequestPayload } from '~/src/server/routes/types.js' + * @import { AnyFormRequest } from '~/src/server/plugins/engine/types.js' */ diff --git a/src/server/routes/types.ts b/src/server/routes/types.ts index d66d18af0..5639840fb 100644 --- a/src/server/routes/types.ts +++ b/src/server/routes/types.ts @@ -1,4 +1,8 @@ -import { type ReqRefDefaults, type Request } from '@hapi/hapi' +import { + type ReqRefDefaults, + type Request, + type ResponseToolkit +} from '@hapi/hapi' import { type FormPayload } from '~/src/server/plugins/engine/types.js' @@ -33,6 +37,7 @@ export interface FormRequestPayloadRefs extends FormRequestRefs { export type FormRequest = Request export type FormRequestPayload = Request +export type FormResponseToolkit = Pick export enum FormAction { Continue = 'continue', @@ -40,7 +45,7 @@ export enum FormAction { Delete = 'delete', AddAnother = 'add-another', Send = 'send', - SaveAndReturn = 'save-and-return' + SaveAndExit = 'save-and-exit' } export enum FormStatus { diff --git a/src/server/schemas/index.ts b/src/server/schemas/index.ts index 38006860c..f181e7f01 100644 --- a/src/server/schemas/index.ts +++ b/src/server/schemas/index.ts @@ -14,7 +14,7 @@ export const actionSchema = Joi.string() FormAction.Delete, FormAction.AddAnother, FormAction.Send, - FormAction.SaveAndReturn + FormAction.SaveAndExit ) .default(FormAction.Validate) .optional() diff --git a/src/server/services/cacheService.test.ts b/src/server/services/cacheService.test.ts index f11c42797..0e58a9fbf 100644 --- a/src/server/services/cacheService.test.ts +++ b/src/server/services/cacheService.test.ts @@ -2,11 +2,7 @@ import { type Request, type Server } from '@hapi/hapi' import { config } from '~/src/config/index.js' import { type FormRequest } from '~/src/server/routes/types.js' -import { - ADDITIONAL_IDENTIFIER, - CacheService, - merge -} from '~/src/server/services/cacheService.js' +import { CacheService, merge } from '~/src/server/services/cacheService.js' describe('CacheService', () => { let mockServer: Partial @@ -76,63 +72,6 @@ describe('CacheService', () => { expect(result).toEqual({}) }) }) - - it('should rehydrate state using custom fetcher when cache is missed', async () => { - const rehydratedState = { rehydrated: true } - - const customFetcher = jest.fn().mockResolvedValue(rehydratedState) - - cacheService = new CacheService({ - server: mockServer as Server, - cacheName: 'test-cache', - options: { sessionHydrator: customFetcher } - }) - - const mockRequest = { - yar: { id: 'session-id' }, - params: { state: 's', slug: 'p' } - } as unknown as FormRequest - - mockCache.get - .mockResolvedValueOnce(null) - .mockResolvedValueOnce(rehydratedState) - - const result = await cacheService.getState(mockRequest) - - expect(customFetcher).toHaveBeenCalledWith(mockRequest) - expect(mockCache.set).toHaveBeenCalledWith( - expect.objectContaining({ - segment: 'cache', - id: expect.stringContaining('session-id') - }), - rehydratedState, - config.get('sessionTimeout') - ) - expect(result).toEqual(rehydratedState) - }) - - it('should return empty object when custom fetcher returns null', async () => { - const customFetcher = jest.fn().mockResolvedValue(null) - - cacheService = new CacheService({ - server: mockServer as Server, - cacheName: 'test-cache', - options: { sessionHydrator: customFetcher } - }) - - const mockRequest = { - yar: { id: 'session-id' }, - params: { state: 's', slug: 'p' } - } as unknown as FormRequest - - mockCache.get.mockResolvedValue(null) - - const result = await cacheService.getState(mockRequest) - - expect(customFetcher).toHaveBeenCalledWith(mockRequest) - expect(mockCache.set).not.toHaveBeenCalled() - expect(result).toEqual({}) - }) }) describe('setState', () => { @@ -178,61 +117,6 @@ describe('CacheService', () => { ) }) }) - - it('should use custom key generator if provided', async () => { - const customKey = 'my-custom-key' - const customKeyGenerator = jest.fn().mockReturnValue(customKey) - - cacheService = new CacheService({ - server: mockServer as Server, - cacheName: 'test-cache', - options: { keyGenerator: customKeyGenerator } - }) - - const mockRequest = { - yar: { id: 'some-session' }, - params: { state: 'form1', slug: 'page1' } - } as unknown as FormRequest - - await cacheService.setState(mockRequest, { test: 'value' }) - - expect(mockCache.set).toHaveBeenCalledWith( - { - segment: 'cache', - id: 'my-custom-key' - }, - { test: 'value' }, - expect.any(Number) - ) - }) - - it('should append additionalIdentifier to custom key', () => { - const customKey = 'custom:key:base:' - const customKeyGenerator = jest.fn().mockReturnValue(customKey) - - cacheService = new CacheService({ - server: mockServer as Server, - cacheName: 'test-cache', - options: { keyGenerator: customKeyGenerator } - }) - - const mockRequest = { - yar: { id: 'session-id' }, - params: { state: 'formA', slug: 'step1' } - } as unknown as FormRequest - - const result = cacheService.Key( - mockRequest, - ADDITIONAL_IDENTIFIER.Confirmation - ) - - expect(result).toEqual({ - segment: 'cache', - id: 'custom:key:base::confirmation' - }) - - expect(customKeyGenerator).toHaveBeenCalledWith(mockRequest) - }) }) describe('merge', () => { diff --git a/src/server/services/cacheService.ts b/src/server/services/cacheService.ts index b8b7f391f..211184b50 100644 --- a/src/server/services/cacheService.ts +++ b/src/server/services/cacheService.ts @@ -1,18 +1,16 @@ -import { type Request, type Server } from '@hapi/hapi' +import { type Server } from '@hapi/hapi' import * as Hoek from '@hapi/hoek' import { config } from '~/src/config/index.js' import { type createServer } from '~/src/server/index.js' import { + type AnyFormRequest, + type AnyRequest, type FormPayload, type FormState, type FormSubmissionError, type FormSubmissionState } from '~/src/server/plugins/engine/types.js' -import { - type FormRequest, - type FormRequestPayload -} from '~/src/server/routes/types.js' const partition = 'cache' @@ -25,66 +23,28 @@ export class CacheService { * This service is responsible for getting, storing or deleting a user's session data in the cache. This service has been registered by {@link createServer} */ cache - generateKey?: (request: Request | FormRequest | FormRequestPayload) => string - customFetcher?: ( - request: Request | FormRequest | FormRequestPayload - ) => Promise - logger: Server['logger'] - constructor({ - server, - cacheName, - options - }: { - server: Server - cacheName?: string - options?: { - keyGenerator?: ( - request: Request | FormRequest | FormRequestPayload - ) => string - sessionHydrator?: ( - request: Request | FormRequest | FormRequestPayload - ) => Promise - } - }) { - const { keyGenerator, sessionHydrator } = options ?? {} + constructor({ server, cacheName }: { server: Server; cacheName?: string }) { if (!cacheName) { server.log( 'warn', 'You are using the default hapi cache. Please provide a cache name in plugin registration options.' ) } - this.generateKey = keyGenerator ?? this.defaultKeyGenerator.bind(this) - this.customFetcher = sessionHydrator ?? undefined + this.cache = server.cache({ cache: cacheName, segment: 'formSubmission' }) this.logger = server.logger } - async getState( - request: Request | FormRequest | FormRequestPayload - ): Promise { + async getState(request: AnyRequest): Promise { const key = this.Key(request) - - let cached = await this.cache.get(key) - - // If nothing in Redis, attempt to rehydrate from backend DB - if (!cached && this.customFetcher) { - const rehydrated = await this.customFetcher(request) - - if (rehydrated != null) { - await this.cache.set(key, rehydrated, config.get('sessionTimeout')) - cached = await this.getState(request) - } - } + const cached = await this.cache.get(key) return cached ?? {} } - async setState( - request: FormRequest | FormRequestPayload, - state: FormSubmissionState - ) { + async setState(request: AnyFormRequest, state: FormSubmissionState) { const key = this.Key(request) const ttl = config.get('sessionTimeout') @@ -94,7 +54,7 @@ export class CacheService { } async getConfirmationState( - request: FormRequest | FormRequestPayload + request: AnyFormRequest ): Promise<{ confirmed?: true }> { const key = this.Key(request, ADDITIONAL_IDENTIFIER.Confirmation) const value = await this.cache.get(key) @@ -103,7 +63,7 @@ export class CacheService { } async setConfirmationState( - request: FormRequest | FormRequestPayload, + request: AnyFormRequest, confirmationState: { confirmed?: true } ) { const key = this.Key(request, ADDITIONAL_IDENTIFIER.Confirmation) @@ -112,14 +72,14 @@ export class CacheService { return this.cache.set(key, confirmationState, ttl) } - async clearState(request: FormRequest | FormRequestPayload) { + async clearState(request: AnyFormRequest) { if (request.yar.id) { await this.cache.drop(this.Key(request)) } } getFlash( - request: FormRequest | FormRequestPayload + request: AnyFormRequest ): { errors: FormSubmissionError[] } | undefined { const key = this.Key(request) const messages = request.yar.flash(key.id) @@ -130,7 +90,7 @@ export class CacheService { } setFlash( - request: FormRequest | FormRequestPayload, + request: AnyFormRequest, message: { errors: FormSubmissionError[] } ) { const key = this.Key(request) @@ -138,35 +98,24 @@ export class CacheService { request.yar.flash(key.id, message) } - private defaultKeyGenerator( - request: Request | FormRequest | FormRequestPayload - ): string { - if (!request.yar.id) { - throw new Error('No session ID found') - } - - const state = (request.params.state as string) || '' - const slug = (request.params.slug as string) || '' - return `${request.yar.id}:${state}:${slug}:` - } - /** * The key used to store user session data against. * If there are multiple forms on the same runner instance, for example `form-a` and `form-a-feedback` this will prevent CacheService from clearing data from `form-a` if a user gave feedback before they finished `form-a` * @param request - hapi request object * @param additionalIdentifier - appended to the id */ - Key( - request: Request | FormRequest | FormRequestPayload, - additionalIdentifier?: ADDITIONAL_IDENTIFIER - ) { - const baseKey = this.generateKey - ? this.generateKey(request) - : this.defaultKeyGenerator(request) + Key(request: AnyRequest, additionalIdentifier?: ADDITIONAL_IDENTIFIER) { + if (!request.yar.id) { + throw new Error('No session ID found') + } + + const state = (request.params.state as string) || '' + const slug = (request.params.slug as string) || '' + const key = `${request.yar.id}:${state}:${slug}:` return { segment: partition, - id: `${baseKey}${additionalIdentifier ?? ''}` + id: `${key}${additionalIdentifier ?? ''}` } } } diff --git a/src/server/types.ts b/src/server/types.ts index 10f50cc3f..b5179ae00 100644 --- a/src/server/types.ts +++ b/src/server/types.ts @@ -4,6 +4,7 @@ import { type SubmitPayload, type SubmitResponsePayload } from '@defra/forms-model' +import { type Server } from '@hapi/hapi' import { type FormModel } from '~/src/server/plugins/engine/models/index.js' import { type DetailItem } from '~/src/server/plugins/engine/models/types.js' @@ -18,6 +19,7 @@ import { type FormRequestPayload, type FormStatus } from '~/src/server/routes/types.js' +import { type CacheService } from '~/src/server/services/cacheService.js' export interface FormsService { getFormMetadata: (slug: string) => Promise @@ -49,7 +51,8 @@ export interface RouteConfig { controllers?: Record preparePageEventRequestOptions?: PreparePageEventRequestOptions onRequest?: OnRequestCallback - saveAndReturn?: PluginOptions['saveAndReturn'] + saveAndExit?: PluginOptions['saveAndExit'] + cacheServiceCreator?: (server: Server) => CacheService } export interface OutputService { diff --git a/src/typings/hapi/index.d.ts b/src/typings/hapi/index.d.ts index 8067b4e43..0c9405fd7 100644 --- a/src/typings/hapi/index.d.ts +++ b/src/typings/hapi/index.d.ts @@ -5,11 +5,10 @@ import { type ServerYar, type Yar } from '@hapi/yar' import { type Logger } from 'pino' import { type FormModel } from '~/src/server/plugins/engine/models/index.js' -import { type PluginOptions } from '~/src/server/plugins/engine/types.ts' import { - type FormRequest, - type FormRequestPayload -} from '~/src/server/routes/types.js' + type AnyFormRequest, + type PluginOptions +} from '~/src/server/plugins/engine/types.ts' import { type CacheService } from '~/src/server/services/index.js' declare module '@hapi/hapi' { @@ -17,15 +16,15 @@ declare module '@hapi/hapi' { // props from plugins which doesn't export @types interface PluginProperties { crumb: { - generate?: (request: Request | FormRequest | FormRequestPayload) => string + generate?: (request: AnyRequest) => string } 'forms-engine-plugin': { baseLayoutPath: string cacheService: CacheService viewContext?: ( - request: FormRequest | FormRequestPayload | null + request: AnyFormRequest | null ) => Record | Promise> - saveAndReturn?: PluginOptions['saveAndReturn'] + saveAndExit?: PluginOptions['saveAndExit'] } } diff --git a/test/form/cacheService.test.js b/test/form/cacheService.test.js new file mode 100644 index 000000000..d71e01f4d --- /dev/null +++ b/test/form/cacheService.test.js @@ -0,0 +1,130 @@ +import { join } from 'path' + +import { Engine as CatboxMemory } from '@hapi/catbox-memory' + +import { FORM_PREFIX } from '~/src/server/constants.js' +import { createServer } from '~/src/server/index.js' +import { CacheService } from '~/src/server/services/cacheService.js' +import { getCookie, getCookieHeader } from '~/test/utils/get-cookie.js' + +const basePath = `${FORM_PREFIX}/minimal` + +class NewCacheService extends CacheService { + /** + * + * @param {AnyRequest} _request + * @param {ADDITIONAL_IDENTIFIER} [_additionalIdentifier] + * @returns + */ + Key(_request, _additionalIdentifier) { + return { + segment: 'irrelevant', + id: 'my-custom-identifier' + } + } +} + +describe('CacheService', () => { + /** @type {Server} */ + let server + + afterEach(async () => { + await server.stop() + }) + + test('the new cache service is utilised', async () => { + // Spy on CatboxMemory.prototype.set globally + const setStateSpy = jest.spyOn(NewCacheService.prototype, 'setState') + const catboxSetSpy = jest.spyOn(CatboxMemory.prototype, 'set') + + server = await createServer({ + formFileName: 'minimal.js', + formFilePath: join(import.meta.dirname, 'definitions'), + cacheServiceCreator: (server) => + new NewCacheService({ server, cacheName: 'session' }) + }) + + await server.initialize() + + // Navigate to start + const headers = undefined + const response = await server.inject({ + url: `${basePath}/start`, + headers + }) + + // Extract the session cookie + const csrfToken = getCookie(response, 'crumb') + const newHeaders = getCookieHeader(response, ['session', 'crumb']) + + // Submit answers + await server.inject({ + url: `${basePath}/start`, + method: 'POST', + headers: newHeaders, + payload: { + crumb: csrfToken, + field: 'value' + } + }) + + // assert our new custom cache is used + expect(setStateSpy).toHaveBeenCalled() + setStateSpy.mockRestore() + + // Assert the custom ID 'my-custom-identifier' is used + expect(catboxSetSpy).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'my-custom-identifier', + segment: 'formSubmission' + }), + expect.any(Object), + expect.any(Number) + ) + catboxSetSpy.mockRestore() + }) + + test('the default cache service is utilised', async () => { + // Spy on CatboxMemory.prototype.set globally + const setStateSpy = jest.spyOn(CacheService.prototype, 'setState') + + server = await createServer({ + formFileName: 'minimal.js', + formFilePath: join(import.meta.dirname, 'definitions') + }) + + await server.initialize() + + // Navigate to start + const headers = undefined + const response = await server.inject({ + url: `${basePath}/start`, + headers + }) + + // Extract the session cookie + const csrfToken = getCookie(response, 'crumb') + const newHeaders = getCookieHeader(response, ['session', 'crumb']) + + // Submit answers + await server.inject({ + url: `${basePath}/start`, + method: 'POST', + headers: newHeaders, + payload: { + crumb: csrfToken, + field: 'value' + } + }) + + // assert our new custom cache is used + expect(setStateSpy).toHaveBeenCalled() + setStateSpy.mockRestore() + }) +}) + +/** + * @import { Server } from '@hapi/hapi' + * @import { AnyFormRequest, AnyRequest, FormSubmissionState } from '~/src/server/plugins/engine/types.js' + * @import { ADDITIONAL_IDENTIFIER } from '~/src/server/services/cacheService.js' + */ diff --git a/test/form/save-and-return.test.js b/test/form/save-and-exit.test.js similarity index 56% rename from test/form/save-and-return.test.js rename to test/form/save-and-exit.test.js index f3cd83989..d527249e1 100644 --- a/test/form/save-and-return.test.js +++ b/test/form/save-and-exit.test.js @@ -4,7 +4,6 @@ import { StatusCodes } from 'http-status-codes' import { FORM_PREFIX } from '~/src/server/constants.js' import { createServer } from '~/src/server/index.js' -import { configureEnginePlugin } from '~/src/server/plugins/engine/configureEnginePlugin.js' import { getFormMetadata } from '~/src/server/plugins/engine/services/formsService.js' import * as fixtures from '~/test/fixtures/index.js' import { renderResponse } from '~/test/helpers/component-helpers.js' @@ -16,7 +15,7 @@ jest.mock('~/src/server/utils/notify.ts') jest.mock('~/src/server/plugins/engine/services/formsService.js') jest.mock('~/src/server/plugins/engine/services/formSubmissionService.js') -describe('Save and Return functionality', () => { +describe('Save and Exit functionality', () => { /** @type {Server} */ let server @@ -27,15 +26,19 @@ describe('Save and Return functionality', () => { let headers beforeAll(async () => { + /** + * @param {FormRequestPayload} request + * @param {FormResponseToolkit} h + */ + function saveAndExit(request, h) { + return h.redirect('/my-save-and-exit') + } + server = await createServer({ formFileName: 'basic.js', formFilePath: join(import.meta.dirname, 'definitions'), enforceCsrf: true, - saveAndReturn: { - keyGenerator: () => 'test-key', - sessionHydrator: () => Promise.resolve({ someState: 'value' }), - sessionPersister: () => Promise.resolve(undefined) - } + saveAndExit }) await server.initialize() @@ -56,29 +59,29 @@ describe('Save and Return functionality', () => { await server.stop() }) - describe('Save and Return button', () => { - it('should render the save and return button on question pages with the correct name and value attributes', async () => { + describe('Save and Exit button', () => { + it('should render the save and exit button on question pages with the correct name and value attributes', async () => { const { container } = await renderResponse(server, { url: `${basePath}/licence`, headers }) const $saveButton = container.getByRole('button', { - name: 'Save and return' + name: 'Save and exit' }) expect($saveButton).toBeInTheDocument() expect($saveButton).toHaveClass('govuk-button--secondary') expect($saveButton).toHaveAttribute('name', 'action') - expect($saveButton).toHaveAttribute('value', 'save-and-return') + expect($saveButton).toHaveAttribute('value', 'save-and-exit') }) }) - describe('Save and Return POST functionality', () => { - it('should save form data and redirect to exit page when action is save-and-return', async () => { + describe('Save and Exit POST functionality', () => { + it('should save form data when action is save-and-exit', async () => { const payload = { licenceLength: '1', - action: 'save-and-return', + action: 'save-and-exit', crumb: csrfToken } @@ -90,7 +93,7 @@ describe('Save and Return functionality', () => { }) expect(response.statusCode).toBe(StatusCodes.MOVED_TEMPORARILY) - expect(response.headers.location).toBe(`${basePath}/exit`) + expect(response.headers.location).toBe('/my-save-and-exit') }) it('should continue normally when action is continue', async () => { @@ -108,38 +111,12 @@ describe('Save and Return functionality', () => { }) expect(response.statusCode).toBe(StatusCodes.SEE_OTHER) - expect(response.headers.location).not.toBe(`${basePath}/exit`) - }) - - it('should work correctly when no saveAndReturn is provided', async () => { - const { options } = await configureEnginePlugin({ - formFileName: 'basic.js', - formFilePath: join(import.meta.dirname, 'definitions') - }) - - expect(options.saveAndReturn).toBeUndefined() - - const payload = { - licenceLength: '1', - action: 'save-and-return', - crumb: csrfToken - } - - const response = await server.inject({ - url: `${basePath}/licence`, - method: 'POST', - headers, - payload - }) - - expect(response.statusCode).toBe(StatusCodes.MOVED_TEMPORARILY) - expect(response.headers.location).toBe(`${basePath}/exit`) }) it('should prevent invalid form state being persisted', async () => { const payload = { licenceLength: '', - action: 'save-and-return', + action: 'save-and-exit', crumb: csrfToken } @@ -150,13 +127,12 @@ describe('Save and Return functionality', () => { payload }) - expect(response.headers.location).not.toBe(`${basePath}/exit`) expect(response.statusCode).not.toBe(StatusCodes.MOVED_TEMPORARILY) // we shouldn't be redirected to the next question }) it('should return 404 for non-existent page', async () => { const payload = { - action: 'save-and-return', + action: 'save-and-exit', crumb: csrfToken } @@ -171,54 +147,11 @@ describe('Save and Return functionality', () => { }) }) - describe('Exit page', () => { - it('should render the exit page with success message', async () => { - const { container } = await renderResponse(server, { - url: `${basePath}/exit`, - headers - }) - - const $heading = container.getByRole('heading', { - level: 1 - }) - - expect($heading).toHaveTextContent('Your progress has been saved') - }) - - it('should render the exit page with return URL when provided', async () => { - const returnUrl = 'https://example.com/return' - const { container } = await renderResponse(server, { - url: `${basePath}/exit?returnUrl=${encodeURIComponent(returnUrl)}`, - headers - }) - - const $returnButton = container.getByRole('button', { - name: 'Return to application' - }) - - expect($returnButton).toBeInTheDocument() - expect($returnButton).toHaveAttribute('href', returnUrl) - }) - - it('should not render return button when no return URL is provided', async () => { - const { container } = await renderResponse(server, { - url: `${basePath}/exit`, - headers - }) - - const $returnButton = container.queryByRole('button', { - name: 'Return to application' - }) - - expect($returnButton).not.toBeInTheDocument() - }) - }) - describe('Error handling', () => { it('should handle CSRF token validation', async () => { const payload = { licenceLength: '1', - action: 'save-and-return', + action: 'save-and-exit', crumb: 'invalid-csrf-token' } @@ -235,7 +168,7 @@ describe('Save and Return functionality', () => { it('should handle missing CSRF token', async () => { const payload = { licenceLength: '1', - action: 'save-and-return' + action: 'save-and-exit' } const response = await server.inject({ @@ -252,4 +185,5 @@ describe('Save and Return functionality', () => { /** * @import { Server } from '@hapi/hapi' + * @import { FormRequestPayload, FormResponseToolkit } from '~/src/server/routes/types.js' */