Merge branch 'main' into feat/df-922-version-metadata-refactor

Author: mokhld

src/server/plugins/engine/models/FormModel.ts

@@ -48,7 +48,6 @@ import {
   createPage,
   type PageControllerClass
 } from '~/src/server/plugins/engine/pageControllers/helpers/pages.js'
-import { copyNotYetValidatedState } from '~/src/server/plugins/engine/pageControllers/helpers/state.js'
 import { validationOptions as opts } from '~/src/server/plugins/engine/pageControllers/validationOptions.js'
 import * as defaultServices from '~/src/server/plugins/engine/services/index.js'
 import {
@@ -402,9 +401,6 @@ export class FormModel {
     // Add paths for navigation
     this.assignPaths(context)
 
-    // Handle restoration of payload from say a 'save-and-exit' request
-    copyNotYetValidatedState(request, context)
-
     return context
   }
 

src/server/plugins/engine/pageControllers/QuestionPageController.ts

@@ -31,10 +31,7 @@ import {
 } 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 {
-  clearNotYetValidatedState,
-  prefillStateFromQueryParameters
-} from '~/src/server/plugins/engine/pageControllers/helpers/state.js'
+import { prefillStateFromQueryParameters } from '~/src/server/plugins/engine/pageControllers/helpers/state.js'
 import {
   type AnyFormRequest,
   type FormContext,
@@ -342,8 +339,7 @@ export class QuestionPageController extends PageController {
 
     const cacheService = getCacheService(request.server)
 
-    // Clear any 'not yet validated' state before saving to cache
-    return cacheService.setState(request, clearNotYetValidatedState(state))
+    return cacheService.setState(request, state)
   }
 
   async mergeState(

src/server/plugins/engine/pageControllers/helpers/state.test.ts

@@ -1,19 +1,27 @@
-import { ComponentType, type Page } from '@defra/forms-model'
+import { ComponentType, ControllerType, type Page } from '@defra/forms-model'
 
 import { type FormModel } from '~/src/server/plugins/engine/models/FormModel.js'
 import { type PageControllerClass } from '~/src/server/plugins/engine/pageControllers/helpers/pages.js'
 import {
+  checkSaveAndExitRepeater,
   copyNotYetValidatedState,
   prefillStateFromQueryParameters,
   stripParam
 } from '~/src/server/plugins/engine/pageControllers/helpers/state.js'
 import {
   type AnyFormRequest,
-  type FormContext,
-  type FormContextRequest
+  type FormContext
 } from '~/src/server/plugins/engine/types.js'
 import { type FormsService, type Services } from '~/src/server/types.js'
 
+const mockGetCacheService = jest.fn()
+const mockCacheService = { setState: jest.fn() }
+
+jest.mock('~/src/server/plugins/engine/helpers.ts', () => ({
+  __esModule: true,
+  getCacheService: (...args: unknown[]) => mockGetCacheService(...args)
+}))
+
 function buildMockPage(
   pagesOverride = {},
   stateOverride = {},
@@ -225,23 +233,26 @@ describe('State helpers', () => {
   })
 
   describe('copyNotYetValidatedState', () => {
-    it('should ignore if no invalid state', () => {
-      const mockRequest = {} as FormContextRequest
+    beforeEach(() => {
+      mockGetCacheService.mockReturnValue(mockCacheService)
+    })
+    it('should ignore if no invalid state', async () => {
+      const mockRequest = {} as AnyFormRequest
       const mockContext = {
         state: { abc: '123' },
         payload: {}
       } as unknown as FormContext
-      copyNotYetValidatedState(mockRequest, mockContext)
+      await copyNotYetValidatedState(mockRequest, mockContext)
       expect(mockContext.state).toEqual({ abc: '123' })
       expect(mockContext.payload).toEqual({})
     })
 
-    it('should ignore if wrong path', () => {
+    it('should ignore if wrong path', async () => {
       const mockRequest = {
         url: {
           pathname: '/form-page1'
         }
-      } as unknown as FormContextRequest
+      } as unknown as AnyFormRequest
       const mockContext = {
         state: {
           abc: '123',
@@ -252,7 +263,7 @@ describe('State helpers', () => {
         },
         payload: {}
       } as unknown as FormContext
-      copyNotYetValidatedState(mockRequest, mockContext)
+      await copyNotYetValidatedState(mockRequest, mockContext)
       expect(mockContext.state).toEqual({
         abc: '123',
         __stateNotYetValidated: {
@@ -263,12 +274,12 @@ describe('State helpers', () => {
       expect(mockContext.payload).toEqual({})
     })
 
-    it('should apply if correct path', () => {
+    it('should apply if correct path', async () => {
       const mockRequest = {
         url: {
           pathname: '/form-page1'
         }
-      } as unknown as FormContextRequest
+      } as unknown as AnyFormRequest
       const mockContext = {
         state: {
           abc: '123',
@@ -279,17 +290,70 @@ describe('State helpers', () => {
         },
         payload: {}
       } as unknown as FormContext
-      copyNotYetValidatedState(mockRequest, mockContext)
+      await copyNotYetValidatedState(mockRequest, mockContext)
       expect(mockContext.state).toEqual({
         abc: '123',
-        __stateNotYetValidated: {
-          def: '456',
-          __currentPagePath: '/form-page1'
-        }
+        __stateNotYetValidated: undefined
       })
       expect(mockContext.payload).toEqual({
         def: '456'
       })
     })
   })
+
+  describe('checkSaveAndExitRepeater', () => {
+    function createMockContextWithPath(path: string) {
+      return {
+        state: {
+          abc: '123',
+          __stateNotYetValidated: {
+            def: '456',
+            __currentPagePath: path
+          }
+        },
+        payload: {}
+      } as unknown as FormContext
+    }
+
+    const mockModel = {
+      def: {
+        pages: [
+          {
+            controller: ControllerType.Repeat,
+            path: '/personal_details'
+          }
+        ]
+      },
+      basePath: 'form/preview/draft/repeater-test'
+    } as unknown as FormModel
+
+    it('should return undefined if url does not end in a guid', () => {
+      const mockContext = createMockContextWithPath(
+        '/form/preview/draft/repeater-test/personal_details'
+      )
+      expect(checkSaveAndExitRepeater(mockContext, mockModel)).toBeUndefined()
+    })
+
+    it('should return undefined if url ends in a guid but not a repeater path', () => {
+      const mockContext = createMockContextWithPath(
+        '/form/preview/draft/repeater-test/wrong_page/7d27fe6e-73e8-4265-84bd-1e118c92470b'
+      )
+      expect(checkSaveAndExitRepeater(mockContext, mockModel)).toBeUndefined()
+    })
+
+    it('should return undefined if url is not a string', () => {
+      // @ts-expect-error - invalid dataype on purpose for this test
+      const mockContext = createMockContextWithPath({})
+      expect(checkSaveAndExitRepeater(mockContext, mockModel)).toBeUndefined()
+    })
+
+    it('should return correct urls if url ends in a guid and is a repeater path', () => {
+      const mockContext = createMockContextWithPath(
+        '/form/preview/draft/repeater-test/personal_details/7d27fe6e-73e8-4265-84bd-1e118c92470b'
+      )
+      expect(checkSaveAndExitRepeater(mockContext, mockModel)).toBe(
+        '/form/preview/draft/repeater-test/personal_details/7d27fe6e-73e8-4265-84bd-1e118c92470b'
+      )
+    })
+  })
 })

src/server/plugins/engine/pageControllers/helpers/state.ts

@@ -1,21 +1,24 @@
-import { getHiddenFields } from '@defra/forms-model'
+import { ControllerType, getHiddenFields } from '@defra/forms-model'
+import { validate as isValidUUID } from 'uuid'
 
+import { getCacheService } from '~/src/server/plugins/engine/helpers.js'
 import {
   CURRENT_PAGE_PATH_KEY,
   STATE_NOT_YET_VALIDATED
 } from '~/src/server/plugins/engine/index.js'
+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 FormStateValue,
-  type FormSubmissionState,
   type FormValue
 } from '~/src/server/plugins/engine/types.js'
 import { type FormQuery } from '~/src/server/routes/types.js'
 import { type Services } from '~/src/server/types.js'
 
+const GUID_LENGTH = 36
+
 /**
  * A series of functions that can transform a pre-fill input parameter e.g lookup a form title based on form id
  */
@@ -100,14 +103,56 @@ export async function prefillStateFromQueryParameters(
   return true
 }
 
+/**
+ * Checks whether the save-and-exit finished on a repeater with partial state
+ * @param context - the form context
+ */
+export function checkSaveAndExitRepeater(
+  context: FormContext,
+  model: FormModel
+) {
+  const potentiallyInvalidState = context.state[STATE_NOT_YET_VALIDATED] as
+    | Record<string, FormValue>
+    | undefined
+  if (!potentiallyInvalidState) {
+    return
+  }
+
+  const originalPath = potentiallyInvalidState[CURRENT_PAGE_PATH_KEY]
+
+  const repeaterPaths = model.def.pages
+    .filter((page) => page.controller === ControllerType.Repeat)
+    .map((p) => `/${model.basePath}${p.path}/`)
+
+  if (typeof originalPath !== 'string') {
+    return undefined
+  }
+
+  const segments = originalPath.split('/')
+  const lastSegment = segments.at(-1) ?? ''
+
+  if (!isValidUUID(lastSegment)) {
+    return undefined
+  }
+
+  const guidStartIndex = originalPath.length - GUID_LENGTH
+  const originalPathWithoutGuid = originalPath.substring(0, guidStartIndex)
+
+  if (!repeaterPaths.includes(originalPathWithoutGuid)) {
+    return undefined
+  }
+
+  return originalPath
+}
+
 /**
  * Copies any potentially invalid state into the payload, and removes those values from state
  * NOTE - this method has a side-effect on 'context.state' and 'context.payload'
  * @param request - the form request
  * @param context - the form context
  */
-export function copyNotYetValidatedState(
-  request: FormContextRequest,
+export async function copyNotYetValidatedState(
+  request: AnyFormRequest,
   context: FormContext
 ) {
   const potentiallyInvalidState = context.state[STATE_NOT_YET_VALIDATED] as
@@ -125,18 +170,13 @@ export function copyNotYetValidatedState(
       ...potentiallyInvalidState,
       [CURRENT_PAGE_PATH_KEY]: undefined
     }
-  }
-}
 
-/**
- * Remove any temporary 'not yet validated' state now that it's been validated
- * @param state - the form state
- */
-export function clearNotYetValidatedState(
-  state: FormSubmissionState
-): FormSubmissionState {
-  if (state[STATE_NOT_YET_VALIDATED]) {
-    state[STATE_NOT_YET_VALIDATED] = undefined
+    // Remove any temporary 'not yet validated' state now it's been copied to the payload
+    if (context.state[STATE_NOT_YET_VALIDATED]) {
+      context.state[STATE_NOT_YET_VALIDATED] = undefined
+    }
+
+    const cacheService = getCacheService(request.server)
+    await cacheService.setState(request, context.state)
   }
-  return state
 }

src/server/plugins/engine/routes/index.test.ts

@@ -86,7 +86,8 @@ describe('redirectOrMakeHandler', () => {
     // Reset mock model
     mockModel.getFormContext = jest.fn().mockReturnValue({
       isForceAccess: false,
-      data: {}
+      data: {},
+      state: {}
     })
 
     // Setup mocks
@@ -225,7 +226,8 @@ describe('redirectOrMakeHandler', () => {
     it('should call makeHandler when context has force access', async () => {
       mockModel.getFormContext = jest.fn().mockReturnValue({
         isForceAccess: true,
-        data: {}
+        data: {},
+        state: {}
       })
 
       await redirectOrMakeHandler(

src/server/plugins/engine/routes/index.ts

@@ -23,6 +23,10 @@ import {
   proceed
 } from '~/src/server/plugins/engine/helpers.js'
 import { type PageControllerClass } from '~/src/server/plugins/engine/pageControllers/helpers/pages.js'
+import {
+  checkSaveAndExitRepeater,
+  copyNotYetValidatedState
+} from '~/src/server/plugins/engine/pageControllers/helpers/state.js'
 import { generateUniqueReference } from '~/src/server/plugins/engine/referenceNumbers.js'
 import * as defaultServices from '~/src/server/plugins/engine/services/index.js'
 import {
@@ -78,6 +82,9 @@ export async function redirectOrMakeHandler(
 
   const flash = cacheService.getFlash(request)
   const context = model.getFormContext(request, state, flash?.errors)
+
+  await copyNotYetValidatedState(request, context)
+
   const relevantPath = page.getRelevantPath(request, context)
   const summaryPath = page.getSummaryPath()
 
@@ -89,14 +96,19 @@ export async function redirectOrMakeHandler(
     }
   }
 
+  // Check whether save-and-exit should resume from within a repeater
+  const resumeInRepeaterUrl = checkSaveAndExitRepeater(context, model)
+  if (resumeInRepeaterUrl) {
+    return proceed(request, h, resumeInRepeaterUrl)
+  }
+
   // Return handler for relevant pages or preview URL direct access
   if (relevantPath.startsWith(page.path) || context.isForceAccess) {
     return makeHandler(page, context)
   }
 
   // Redirect back to last relevant page
   const redirectTo = findPage(model, relevantPath)
-
   // Set the return URL unless an exit page
   if (redirectTo?.next.length) {
     request.query.returnUrl = page.getHref(summaryPath)