fix/df-789: Fixed guards + plugin bump (#1238)

* Fixed guards + plugin bump

* No need for guard here

* Amended comments/methodNames as per review suggestions

Author: jbarnsley10

package-lock.json

@@ -46,7 +46,7 @@
   "license": "SEE LICENSE IN LICENSE",
   "dependencies": {
     "@aws-sdk/client-sns": "^3.997.0",
-    "@defra/forms-engine-plugin": "^4.20.1",
+    "@defra/forms-engine-plugin": "^4.21.0",
     "@defra/forms-model": "^3.0.681",
     "@defra/hapi-tracing": "^1.30.0",
     "@elastic/ecs-pino-format": "^1.5.0",

package.json

@@ -2,7 +2,7 @@ import { FormStatus } from '@defra/forms-engine-plugin/types'
 import Boom from '@hapi/boom'
 
 import { createErrorPreviewModel } from '~/src/server/plugins/error-preview/error-preview-helper.js'
-import { getFormMetadata } from '~/src/server/services/formMetadataGuards.js'
+import { getFormMetadataWithoutGuard } from '~/src/server/services/formMetadataGuards.js'
 import { getFormDefinition } from '~/src/server/services/formsService.js'
 
 /**
@@ -13,7 +13,7 @@ export async function getErrorPreviewHandler(request, h) {
   const { params } = request
   const { slug, path, itemId } = params
 
-  const metadata = await getFormMetadata(slug)
+  const metadata = await getFormMetadataWithoutGuard(slug)
   const definition = await getFormDefinition(metadata.id, FormStatus.Draft)
   if (!definition) {
     throw Boom.notFound(

src/server/plugins/error-preview/error-preview.js

@@ -34,7 +34,7 @@ import {
   publicRoutes,
   saveAndExitRoutes
 } from '~/src/server/routes/index.js'
-import { getFormMetadata } from '~/src/server/services/formMetadataGuards.js'
+import { getFormMetadataWithoutGuard } from '~/src/server/services/formMetadataGuards.js'
 import { getFormDefinition } from '~/src/server/services/formsService.js'
 import { getFeedbackFormLink } from '~/src/server/utils/utils.js'
 
@@ -122,7 +122,7 @@ export default {
         path: '/help/get-support/{slug}',
         async handler(request, h) {
           const { slug } = request.params
-          const form = await getFormMetadata(slug)
+          const form = await getFormMetadataWithoutGuard(slug)
 
           return h.view('help/get-support', { form })
         },
@@ -134,7 +134,7 @@ export default {
         path: '/help/privacy/{slug}',
         async handler(request, h) {
           const { slug } = request.params
-          const form = await getFormMetadata(slug)
+          const form = await getFormMetadataWithoutGuard(slug)
           // It's most likely that we come into this route from a live version of the form
           // so prefer that and fallback to draft if no live version (it is possible to have
           // a live version and no draft version, so we cannot just default to 'draft').
@@ -159,7 +159,7 @@ export default {
         path: '/help/privacy-specific/{slug}',
         async handler(request, h) {
           const { slug } = request.params
-          const form = await getFormMetadata(slug)
+          const form = await getFormMetadataWithoutGuard(slug)
           const formStatus = form.live ? FormStatus.Live : FormStatus.Draft
           const definition = await getFormDefinition(form.id, formStatus)
 
@@ -178,7 +178,7 @@ export default {
         path: '/help/cookies/{slug}',
         async handler(request, h) {
           const { slug } = request.params
-          await getFormMetadata(slug)
+          await getFormMetadataWithoutGuard(slug)
 
           const sessionTimeout = config.get('sessionTimeout')
 
@@ -315,7 +315,7 @@ export default {
         path: '/help/accessibility-statement/{slug}',
         async handler(request, h) {
           const { slug } = request.params
-          await getFormMetadata(slug)
+          await getFormMetadataWithoutGuard(slug)
 
           return h.view('help/accessibility-statement')
         },

src/server/plugins/router.ts

@@ -6,7 +6,7 @@ import {
 } from '@defra/forms-engine-plugin'
 import { getCacheService } from '@defra/forms-engine-plugin/engine/helpers.js'
 import { stateSchema } from '@defra/forms-engine-plugin/schema.js'
-import { slugSchema } from '@defra/forms-model'
+import { FormStatus, slugSchema } from '@defra/forms-model'
 import Boom from '@hapi/boom'
 import * as Hoek from '@hapi/hoek'
 import { StatusCodes } from 'http-status-codes'
@@ -34,8 +34,8 @@ import {
   hasState
 } from '~/src/server/routes/save-and-exit-helper.js'
 import {
-  getFormMetadata,
-  getFormMetadataById
+  getFormMetadataById,
+  getFormMetadataWithGuard
 } from '~/src/server/services/formMetadataGuards.js'
 import {
   getSaveAndExitDetails,
@@ -83,7 +83,7 @@ export default [
     async handler(request, h) {
       const { params } = request
       const { slug, state: status } = params
-      const metadata = await getFormMetadata(slug)
+      const metadata = await getFormMetadataWithGuard(slug, status)
       const model = detailsViewModel(metadata, status)
 
       // Store any outstanding data from the current page in a special attribute
@@ -153,7 +153,7 @@ export default [
       const { email, securityQuestion, securityAnswer } = payload
       // Throws the offline marker BEFORE publishSaveAndExitEvent so we never
       // emit a magic-link email for a form the user can no longer reach.
-      const metadata = await getFormMetadata(slug)
+      const metadata = await getFormMetadataWithGuard(slug, status)
 
       const cacheService = getCacheService(request.server)
 
@@ -206,7 +206,7 @@ export default [
         async failAction(request, h, err) {
           const { params, payload } = request
           const { slug, state: status } = params
-          const metadata = await getFormMetadata(slug)
+          const metadata = await getFormMetadataWithGuard(slug, status)
 
           const model = detailsViewModel(
             metadata,
@@ -231,7 +231,7 @@ export default [
     async handler(request, h) {
       const { params } = request
       const { slug, state: status } = params
-      const metadata = await getFormMetadata(slug)
+      const metadata = await getFormMetadataWithGuard(slug, status)
 
       // Get the email from session
       const email = /** @type {string} */ (
@@ -348,13 +348,13 @@ export default [
     path: '/resume-form-verify/{formId}/{magicLinkId}/{slug}/{state?}',
     async handler(request, h) {
       const { params } = request
-      const { formId, magicLinkId } = params
+      const { formId, magicLinkId, state } = params
 
       // Assert the form is online BEFORE looking up save-and-exit details so
       // we don't leak magic-link validity timing for offline forms.
       let form
       try {
-        form = await getFormMetadataById(formId)
+        form = await getFormMetadataById(formId, state)
       } catch (err) {
         if (isOfflineBoom(err)) {
           throw err
@@ -398,7 +398,7 @@ export default [
 
       if (slug) {
         try {
-          await getFormMetadata(slug)
+          await getFormMetadataWithGuard(slug, FormStatus.Live)
         } catch (err) {
           if (isOfflineBoom(err)) {
             throw err
@@ -433,12 +433,12 @@ export default [
     path: '/resume-form-verify/{formId}/{magicLinkId}/{slug}/{state?}',
     async handler(request, h) {
       const { params, payload } = request
-      const { formId, magicLinkId } = params
+      const { formId, magicLinkId, state } = params
       const { securityAnswer } = payload
 
       let form
       try {
-        form = await getFormMetadataById(formId)
+        form = await getFormMetadataById(formId, state)
       } catch (err) {
         if (isOfflineBoom(err)) {
           throw err
@@ -516,7 +516,10 @@ export default [
             return h.redirect(ERROR_BASE_URL).takeover()
           }
 
-          const form = await getFormMetadataById(resumeDetails.form.id)
+          const form = await getFormMetadataById(
+            resumeDetails.form.id,
+            params.state
+          )
 
           const model = passwordViewModel(
             form,
@@ -532,15 +535,15 @@ export default [
     }
   }),
   /**
-   * @satisfies {ServerRoute<{ Params: { slug: string, state?: string} }>}
+   * @satisfies {ServerRoute<{ Params: { slug: string, state?: FormStatus} }>}
    */
   ({
     method: 'GET',
     path: '/resume-form-success/{slug}/{state?}',
     async handler(request, h) {
       const { params } = request
       const { slug, state } = params
-      const form = await getFormMetadata(slug)
+      const form = await getFormMetadataWithGuard(slug, state)
 
       const model = resumeSuccessViewModel(
         form,
@@ -565,6 +568,5 @@ export default [
 /**
  * @import { ServerRoute } from '@hapi/hapi'
  * @import { CacheRequest, FormPayload } from '@defra/forms-engine-plugin/engine/types.js'
- * @import { FormStatus } from '@defra/forms-model'
  * @import { BoomErrorCustomSaveAndExit, SaveAndExitParams, SaveAndExitPayload, SaveAndExitResumePasswordPayload, SaveAndExitResumePasswordParams } from '~/src/server/models/save-and-exit.js'
  */

src/server/routes/save-and-exit.js

@@ -8,8 +8,8 @@ import { createJoiError } from '~/src/server/helpers/error-helper.js'
 import { createServer } from '~/src/server/index.js'
 import { addError } from '~/src/server/routes/save-and-exit.js'
 import {
-  getFormMetadata,
-  getFormMetadataById
+  getFormMetadataById,
+  getFormMetadataWithGuard
 } from '~/src/server/services/formMetadataGuards.js'
 import {
   getSaveAndExitDetails,
@@ -414,7 +414,7 @@ describe('Save-and-exit check routes', () => {
         statusCode: 503,
         data: { offline: true }
       })
-      jest.mocked(getFormMetadata).mockRejectedValueOnce(offlineErr)
+      jest.mocked(getFormMetadataWithGuard).mockRejectedValueOnce(offlineErr)
       jest.mocked(isOfflineBoom).mockReturnValueOnce(true)
 
       const options = {
@@ -428,7 +428,7 @@ describe('Save-and-exit check routes', () => {
 
     test('logs info on other metadata fetch error', async () => {
       const otherErr = new Error('fetch failed')
-      jest.mocked(getFormMetadata).mockRejectedValueOnce(otherErr)
+      jest.mocked(getFormMetadataWithGuard).mockRejectedValueOnce(otherErr)
       jest.mocked(isOfflineBoom).mockReturnValueOnce(false)
 
       const options = {
@@ -449,7 +449,7 @@ describe('Save-and-exit check routes', () => {
   describe('GET /resume-form-success', () => {
     test('route renders page without state', async () => {
       jest
-        .mocked(getFormMetadata)
+        .mocked(getFormMetadataWithGuard)
         // @ts-expect-error - allow partial objects for tests
         .mockResolvedValueOnce({
           slug: 'my-form-to-resume',
@@ -478,7 +478,7 @@ describe('Save-and-exit check routes', () => {
 
     test('route renders page with slug', async () => {
       jest
-        .mocked(getFormMetadata)
+        .mocked(getFormMetadataWithGuard)
         // @ts-expect-error - allow partial objects for tests
         .mockResolvedValueOnce({
           slug: 'my-form-to-resume',

src/server/routes/save-and-exit.test.js

@@ -1,25 +1,37 @@
 import { assertFormAvailable } from '@defra/forms-engine-plugin'
+import { FormStatus } from '@defra/forms-model'
 
 import * as rawFormsService from '~/src/server/services/formsService.js'
 
 /**
- * Fetch form metadata by slug. Throws the offline marker when the form has
- * been taken offline so route handlers don't have to check.
+ * Fetch form metadata by slug without the 'unavailable' guard.
  * @param {string} slug
  */
-export async function getFormMetadata(slug) {
+export async function getFormMetadataWithoutGuard(slug) {
   const metadata = await rawFormsService.getFormMetadata(slug)
-  assertFormAvailable(metadata)
+  return metadata
+}
+
+/**
+ * Fetch form metadata by slug with 'unavailable' guard (throws the offline marker when the form has
+ * been taken offline so route handlers don't have to check).
+ * @param {string} slug
+ * @param { FormStatus | undefined } formStatus
+ */
+export async function getFormMetadataWithGuard(slug, formStatus) {
+  const metadata = await rawFormsService.getFormMetadata(slug)
+  assertFormAvailable(metadata, formStatus ?? FormStatus.Live, false)
   return metadata
 }
 
 /**
  * Fetch form metadata by id. Throws the offline marker when the form has
  * been taken offline.
  * @param {string} formId
+ * @param {FormStatus} [formStatus]
  */
-export async function getFormMetadataById(formId) {
+export async function getFormMetadataById(formId, formStatus) {
   const metadata = await rawFormsService.getFormMetadataById(formId)
-  assertFormAvailable(metadata)
+  assertFormAvailable(metadata, formStatus ?? FormStatus.Live, false)
   return metadata
 }