From e7de34633ca5d15ee3906a07609fa9910137dd88 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Thu, 4 Dec 2025 11:46:11 +0000 Subject: [PATCH 1/4] Stash - unit test failing --- src/server/plugins/engine/helpers-2.test.ts | 99 +++++++++++++++++++++ src/server/plugins/engine/helpers.test.ts | 1 + src/server/plugins/engine/helpers.ts | 5 +- 3 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 src/server/plugins/engine/helpers-2.test.ts diff --git a/src/server/plugins/engine/helpers-2.test.ts b/src/server/plugins/engine/helpers-2.test.ts new file mode 100644 index 000000000..c4af773ef --- /dev/null +++ b/src/server/plugins/engine/helpers-2.test.ts @@ -0,0 +1,99 @@ +import { + ComponentType, + ControllerType, + type FormDefinition, + type PageQuestion +} from '@defra/forms-model' + +import { setPageTitles } from '~/src/server/plugins/engine/helpers.js' + +const mockInfo = jest.fn() + +const mockLogger = { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn() +} + +jest.mock('~/src/server/common/helpers/logging/logger.ts', () => ({ + createLogger: () => ({ + error: jest.fn(), + info: jest.fn(), + warn: jest.fn() + }) +})) + +describe('setPageTitles', () => { + const definition: FormDefinition = { + name: 'Test Form', + startPage: '/page1', + pages: [ + { + path: '/page1', + title: '', + next: [], + components: [ + { + type: ComponentType.TextField, + name: 'textfield1', + title: 'What is your name?', + options: {}, + schema: {} + }, + { + type: ComponentType.TextField, + name: 'textfield2', + title: 'What is your favourite food?', + options: {}, + schema: {} + } + ] + } satisfies PageQuestion + ], + lists: [], + sections: [], + conditions: [] + } + + beforeEach(() => { + jest.clearAllMocks() + jest.mock('~/src/server/common/helpers/logging/logger.ts', () => ({ + createLogger: jest.fn().mockReturnValue(mockLogger) + })) + }) + it('should set title if missing', () => { + const def = structuredClone(definition) + setPageTitles(def) + expect(def.pages[0].title).toBe('What is your name?') + expect(mockLogger.info).not.toHaveBeenCalled() + }) + + it('should keep title if supplied', () => { + const def = structuredClone(definition) + def.pages[0].title = 'Page 1 title' + setPageTitles(def) + expect(def.pages[0].title).toBe('Page 1 title') + expect(mockLogger.info).not.toHaveBeenCalled() + }) + + it('should log if missing title and no components and not Summary page', () => { + const def = structuredClone(definition) + if ('components' in def.pages[0]) { + def.pages[0].components = [] + } + setPageTitles(def) + expect(def.pages[0].title).toBe('') + expect(mockLogger.info).toHaveBeenCalled() + }) + + it('should not log missing title if Summary page', () => { + const def = structuredClone(definition) + def.pages[0].controller = ControllerType.Summary + if ('components' in def.pages[0]) { + def.pages[0].components = [] + } + setPageTitles(def) + expect(def.pages[0].title).toBe('') + expect(mockInfo).not.toHaveBeenCalled() + }) +}) diff --git a/src/server/plugins/engine/helpers.test.ts b/src/server/plugins/engine/helpers.test.ts index 6d8abc603..10831dbb9 100644 --- a/src/server/plugins/engine/helpers.test.ts +++ b/src/server/plugins/engine/helpers.test.ts @@ -51,6 +51,7 @@ describe('Helpers', () => { let h: FormResponseToolkit beforeEach(() => { + jest.clearAllMocks() const model = new FormModel(definition, { basePath: 'test' }) diff --git a/src/server/plugins/engine/helpers.ts b/src/server/plugins/engine/helpers.ts index 43a934562..b29a4d7ee 100644 --- a/src/server/plugins/engine/helpers.ts +++ b/src/server/plugins/engine/helpers.ts @@ -4,6 +4,7 @@ import { getErrorMessage, hasComponents, isFormType, + isSummaryPage, type ComponentDef, type FormDefinition, type Page @@ -37,7 +38,7 @@ import { type FormResponseToolkit } from '~/src/server/routes/types.js' -const logger = createLogger() +export const logger = createLogger() export const engine = new Liquid({ outputEscape: 'escape', @@ -414,7 +415,7 @@ export function setPageTitles(def: FormDefinition) { page.title = firstFormComponent?.title ?? '' } - if (!page.title) { + if (!page.title && !isSummaryPage(page)) { const formNameMsg = def.name ? ` in form '${def.name}'` : '' logger.info( From 48856a6f1ecf0b8e32449ef525a0bd549a4cc1a8 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Thu, 4 Dec 2025 11:47:53 +0000 Subject: [PATCH 2/4] STash2 --- src/server/plugins/engine/helpers-2.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/server/plugins/engine/helpers-2.test.ts b/src/server/plugins/engine/helpers-2.test.ts index c4af773ef..6b12721ac 100644 --- a/src/server/plugins/engine/helpers-2.test.ts +++ b/src/server/plugins/engine/helpers-2.test.ts @@ -7,8 +7,6 @@ import { import { setPageTitles } from '~/src/server/plugins/engine/helpers.js' -const mockInfo = jest.fn() - const mockLogger = { info: jest.fn(), warn: jest.fn(), @@ -94,6 +92,6 @@ describe('setPageTitles', () => { } setPageTitles(def) expect(def.pages[0].title).toBe('') - expect(mockInfo).not.toHaveBeenCalled() + expect(mockLogger.info).not.toHaveBeenCalled() }) }) From e5e0d94920e299780dabf3205217ad1aeab51ce2 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Thu, 4 Dec 2025 12:24:16 +0000 Subject: [PATCH 3/4] Fixed mock --- src/server/plugins/engine/helpers-2.test.ts | 97 --------------------- src/server/plugins/engine/helpers.test.ts | 94 ++++++++++++++++++++ src/server/plugins/engine/helpers.ts | 2 +- 3 files changed, 95 insertions(+), 98 deletions(-) delete mode 100644 src/server/plugins/engine/helpers-2.test.ts diff --git a/src/server/plugins/engine/helpers-2.test.ts b/src/server/plugins/engine/helpers-2.test.ts deleted file mode 100644 index 6b12721ac..000000000 --- a/src/server/plugins/engine/helpers-2.test.ts +++ /dev/null @@ -1,97 +0,0 @@ -import { - ComponentType, - ControllerType, - type FormDefinition, - type PageQuestion -} from '@defra/forms-model' - -import { setPageTitles } from '~/src/server/plugins/engine/helpers.js' - -const mockLogger = { - info: jest.fn(), - warn: jest.fn(), - error: jest.fn() -} - -jest.mock('~/src/server/common/helpers/logging/logger.ts', () => ({ - createLogger: () => ({ - error: jest.fn(), - info: jest.fn(), - warn: jest.fn() - }) -})) - -describe('setPageTitles', () => { - const definition: FormDefinition = { - name: 'Test Form', - startPage: '/page1', - pages: [ - { - path: '/page1', - title: '', - next: [], - components: [ - { - type: ComponentType.TextField, - name: 'textfield1', - title: 'What is your name?', - options: {}, - schema: {} - }, - { - type: ComponentType.TextField, - name: 'textfield2', - title: 'What is your favourite food?', - options: {}, - schema: {} - } - ] - } satisfies PageQuestion - ], - lists: [], - sections: [], - conditions: [] - } - - beforeEach(() => { - jest.clearAllMocks() - jest.mock('~/src/server/common/helpers/logging/logger.ts', () => ({ - createLogger: jest.fn().mockReturnValue(mockLogger) - })) - }) - it('should set title if missing', () => { - const def = structuredClone(definition) - setPageTitles(def) - expect(def.pages[0].title).toBe('What is your name?') - expect(mockLogger.info).not.toHaveBeenCalled() - }) - - it('should keep title if supplied', () => { - const def = structuredClone(definition) - def.pages[0].title = 'Page 1 title' - setPageTitles(def) - expect(def.pages[0].title).toBe('Page 1 title') - expect(mockLogger.info).not.toHaveBeenCalled() - }) - - it('should log if missing title and no components and not Summary page', () => { - const def = structuredClone(definition) - if ('components' in def.pages[0]) { - def.pages[0].components = [] - } - setPageTitles(def) - expect(def.pages[0].title).toBe('') - expect(mockLogger.info).toHaveBeenCalled() - }) - - it('should not log missing title if Summary page', () => { - const def = structuredClone(definition) - def.pages[0].controller = ControllerType.Summary - if ('components' in def.pages[0]) { - def.pages[0].components = [] - } - setPageTitles(def) - expect(def.pages[0].title).toBe('') - expect(mockLogger.info).not.toHaveBeenCalled() - }) -}) diff --git a/src/server/plugins/engine/helpers.test.ts b/src/server/plugins/engine/helpers.test.ts index 10831dbb9..1ee73d974 100644 --- a/src/server/plugins/engine/helpers.test.ts +++ b/src/server/plugins/engine/helpers.test.ts @@ -1,3 +1,9 @@ +import { + ComponentType, + ControllerType, + type FormDefinition, + type PageQuestion +} from '@defra/forms-model' import Boom from '@hapi/boom' import { type ResponseObject, type ResponseToolkit } from '@hapi/hapi' import { StatusCodes } from 'http-status-codes' @@ -14,6 +20,7 @@ import { getPageHref, proceed, safeGenerateCrumb, + setPageTitles, type GlobalScope } from '~/src/server/plugins/engine/helpers.js' import { handleLegacyRedirect } from '~/src/server/plugins/engine/helpers.js' @@ -45,6 +52,21 @@ interface NunjucksContext { type EvaluateFilter = (this: NunjucksContext, template: unknown) => unknown type HrefFilter = (this: NunjucksContext, path: string) => string | undefined +const mockLoggerError = jest.fn() +const mockLoggerInfo = jest.fn() +const mockLoggerWarn = jest.fn() + +jest.mock('~/src/server/common/helpers/logging/logger.ts', () => ({ + createLogger: jest.fn().mockReturnValue({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + error: () => mockLoggerError(), + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + warn: () => mockLoggerWarn(), + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + info: () => mockLoggerInfo() + }) +})) + describe('Helpers', () => { let page: PageControllerClass let request: FormContextRequest @@ -844,4 +866,76 @@ describe('Helpers', () => { expect(response).toBe(mockRedirectResponse) }) }) + + describe('setPageTitles', () => { + const definition: FormDefinition = { + name: 'Test Form', + startPage: '/page1', + pages: [ + { + path: '/page1', + title: '', + next: [], + components: [ + { + type: ComponentType.TextField, + name: 'textfield1', + title: 'What is your name?', + options: {}, + schema: {} + }, + { + type: ComponentType.TextField, + name: 'textfield2', + title: 'What is your favourite food?', + options: {}, + schema: {} + } + ] + } satisfies PageQuestion + ], + lists: [], + sections: [], + conditions: [] + } + + beforeEach(() => { + jest.clearAllMocks() + }) + it('should set title if missing', () => { + const def = structuredClone(definition) + setPageTitles(def) + expect(def.pages[0].title).toBe('What is your name?') + expect(mockLoggerInfo).not.toHaveBeenCalled() + }) + + it('should keep title if supplied', () => { + const def = structuredClone(definition) + def.pages[0].title = 'Page 1 title' + setPageTitles(def) + expect(def.pages[0].title).toBe('Page 1 title') + expect(mockLoggerInfo).not.toHaveBeenCalled() + }) + + it('should log if missing title and no components and not Summary page', () => { + const def = structuredClone(definition) + if ('components' in def.pages[0]) { + def.pages[0].components = [] + } + setPageTitles(def) + expect(def.pages[0].title).toBe('') + expect(mockLoggerInfo).toHaveBeenCalled() + }) + + it('should not log missing title if Summary page', () => { + const def = structuredClone(definition) + def.pages[0].controller = ControllerType.Summary + if ('components' in def.pages[0]) { + def.pages[0].components = [] + } + setPageTitles(def) + expect(def.pages[0].title).toBe('') + expect(mockLoggerInfo).not.toHaveBeenCalled() + }) + }) }) diff --git a/src/server/plugins/engine/helpers.ts b/src/server/plugins/engine/helpers.ts index b29a4d7ee..3ef1548f1 100644 --- a/src/server/plugins/engine/helpers.ts +++ b/src/server/plugins/engine/helpers.ts @@ -38,7 +38,7 @@ import { type FormResponseToolkit } from '~/src/server/routes/types.js' -export const logger = createLogger() +const logger = createLogger() export const engine = new Liquid({ outputEscape: 'escape', From 685c514a600504e4274eb83628a1e1b1aae13536 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Fri, 5 Dec 2025 09:46:47 +0000 Subject: [PATCH 4/4] Removed logging --- src/server/plugins/engine/helpers.test.ts | 39 ----------------------- src/server/plugins/engine/helpers.ts | 9 ------ 2 files changed, 48 deletions(-) diff --git a/src/server/plugins/engine/helpers.test.ts b/src/server/plugins/engine/helpers.test.ts index 1ee73d974..dd9b765c3 100644 --- a/src/server/plugins/engine/helpers.test.ts +++ b/src/server/plugins/engine/helpers.test.ts @@ -1,6 +1,5 @@ import { ComponentType, - ControllerType, type FormDefinition, type PageQuestion } from '@defra/forms-model' @@ -52,21 +51,6 @@ interface NunjucksContext { type EvaluateFilter = (this: NunjucksContext, template: unknown) => unknown type HrefFilter = (this: NunjucksContext, path: string) => string | undefined -const mockLoggerError = jest.fn() -const mockLoggerInfo = jest.fn() -const mockLoggerWarn = jest.fn() - -jest.mock('~/src/server/common/helpers/logging/logger.ts', () => ({ - createLogger: jest.fn().mockReturnValue({ - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - error: () => mockLoggerError(), - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - warn: () => mockLoggerWarn(), - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - info: () => mockLoggerInfo() - }) -})) - describe('Helpers', () => { let page: PageControllerClass let request: FormContextRequest @@ -906,7 +890,6 @@ describe('Helpers', () => { const def = structuredClone(definition) setPageTitles(def) expect(def.pages[0].title).toBe('What is your name?') - expect(mockLoggerInfo).not.toHaveBeenCalled() }) it('should keep title if supplied', () => { @@ -914,28 +897,6 @@ describe('Helpers', () => { def.pages[0].title = 'Page 1 title' setPageTitles(def) expect(def.pages[0].title).toBe('Page 1 title') - expect(mockLoggerInfo).not.toHaveBeenCalled() - }) - - it('should log if missing title and no components and not Summary page', () => { - const def = structuredClone(definition) - if ('components' in def.pages[0]) { - def.pages[0].components = [] - } - setPageTitles(def) - expect(def.pages[0].title).toBe('') - expect(mockLoggerInfo).toHaveBeenCalled() - }) - - it('should not log missing title if Summary page', () => { - const def = structuredClone(definition) - def.pages[0].controller = ControllerType.Summary - if ('components' in def.pages[0]) { - def.pages[0].components = [] - } - setPageTitles(def) - expect(def.pages[0].title).toBe('') - expect(mockLoggerInfo).not.toHaveBeenCalled() }) }) }) diff --git a/src/server/plugins/engine/helpers.ts b/src/server/plugins/engine/helpers.ts index 3ef1548f1..8cfaa21da 100644 --- a/src/server/plugins/engine/helpers.ts +++ b/src/server/plugins/engine/helpers.ts @@ -4,7 +4,6 @@ import { getErrorMessage, hasComponents, isFormType, - isSummaryPage, type ComponentDef, type FormDefinition, type Page @@ -414,14 +413,6 @@ export function setPageTitles(def: FormDefinition) { page.title = firstFormComponent?.title ?? '' } - - if (!page.title && !isSummaryPage(page)) { - const formNameMsg = def.name ? ` in form '${def.name}'` : '' - - logger.info( - `[pageTitleMissing] Page '${page.path}' has no title${formNameMsg}` - ) - } } }) }