From b800618cc89e604370db4091f521be1fe18131da Mon Sep 17 00:00:00 2001 From: Parvesh M Date: Wed, 28 Jan 2026 13:52:48 +0000 Subject: [PATCH 1/2] feat: ignores 404 errors for live and draft form definition --- designer/server/src/routes/admin/index.js | 16 +++- .../server/src/routes/admin/index.test.js | 78 ++++++++++++++++++- 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/designer/server/src/routes/admin/index.js b/designer/server/src/routes/admin/index.js index 2674fa2ef..64b218543 100644 --- a/designer/server/src/routes/admin/index.js +++ b/designer/server/src/routes/admin/index.js @@ -271,15 +271,27 @@ async function downloadAllFormsAsZip(request, responseToolkit) { */ async function getFormDefinitions(id, includeDraft, token) { const [liveDefinition, draftDefinition] = await Promise.all([ - forms.getLiveFormDefinition(id, token), + forms.getLiveFormDefinition(id, token).catch(ignore404Error), includeDraft - ? forms.getDraftFormDefinition(id, token) + ? forms.getDraftFormDefinition(id, token).catch(ignore404Error) : Promise.resolve(null) ]) return { liveDefinition, draftDefinition } } +/** + * + * @param {Error & {statusCode: StatusCodes}} err + * @returns {null | never} + */ +function ignore404Error(err) { + if (err.statusCode === StatusCodes.NOT_FOUND) { + return null + } + throw err +} + /** * Append form definitions using id-based folder paths * @param {string} id form ID diff --git a/designer/server/src/routes/admin/index.test.js b/designer/server/src/routes/admin/index.test.js index 2c26ec762..eca6dc853 100644 --- a/designer/server/src/routes/admin/index.test.js +++ b/designer/server/src/routes/admin/index.test.js @@ -362,7 +362,7 @@ describe('System admin routes', () => { ) }) - test('should throw error if one of the api calls fails', async () => { + test('should throw error if one of the api calls fails (non 404 error)', async () => { const form1 = Promise.resolve(testFormMetadata) const form2 = Promise.resolve({ ...testFormMetadata, @@ -406,6 +406,82 @@ describe('System admin routes', () => { expect(forms.getDraftFormDefinition).toHaveBeenCalledTimes(1) }) + test('should ignore 404 when fetching live definition', async () => { + const form1 = Promise.resolve(testFormMetadata) + + jest.mocked(forms.listAll).mockImplementationOnce(async function* () { + yield await form1 + }) + + const notFoundError = Object.assign(new Error('Not found'), { + statusCode: StatusCodes.NOT_FOUND + }) + + jest + .mocked(forms.getLiveFormDefinition) + .mockRejectedValueOnce(notFoundError) + + jest + .mocked(forms.getDraftFormDefinition) + .mockResolvedValueOnce(testFormDefinitionWithSinglePage) + + const options = { + method: 'post', + url: '/admin/index', + auth, + payload: { action: 'download' } + } + + const response = await server.inject(options) + + expect(response.statusCode).toBe(StatusCodes.OK) + expect(publishFormsBackupRequestedEvent).toHaveBeenCalledWith( + expect.any(Object), + 1, + expect.any(Number) + ) + expect(forms.getLiveFormDefinition).toHaveBeenCalledTimes(1) + expect(forms.getDraftFormDefinition).toHaveBeenCalledTimes(1) + }) + + test('should ignore 404 when fetching draft definition', async () => { + const form1 = Promise.resolve(testFormMetadata) + + jest.mocked(forms.listAll).mockImplementationOnce(async function* () { + yield await form1 + }) + + const notFoundError = Object.assign(new Error('Not found'), { + statusCode: StatusCodes.NOT_FOUND + }) + + jest + .mocked(forms.getLiveFormDefinition) + .mockResolvedValueOnce(testFormDefinitionWithSinglePage) + + jest + .mocked(forms.getDraftFormDefinition) + .mockRejectedValueOnce(notFoundError) + + const options = { + method: 'post', + url: '/admin/index', + auth, + payload: { action: 'download' } + } + + const response = await server.inject(options) + + expect(response.statusCode).toBe(StatusCodes.OK) + expect(publishFormsBackupRequestedEvent).toHaveBeenCalledWith( + expect.any(Object), + 1, + expect.any(Number) + ) + expect(forms.getLiveFormDefinition).toHaveBeenCalledTimes(1) + expect(forms.getDraftFormDefinition).toHaveBeenCalledTimes(1) + }) + test('should process forms in batches', async () => { // Create 12 forms to test batching (concurrency is 5 in the code) const mockForms = Promise.resolve( From 2f28ca255e89b26fa435e2aab4396384e6f346d5 Mon Sep 17 00:00:00 2001 From: Parvesh M Date: Wed, 28 Jan 2026 16:15:46 +0000 Subject: [PATCH 2/2] feat: ignores 404 errors for live and draft form definition --- designer/server/src/routes/admin/index.js | 25 +++++++------------ .../server/src/routes/admin/index.test.js | 20 ++++++++++----- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/designer/server/src/routes/admin/index.js b/designer/server/src/routes/admin/index.js index 1ba301cd6..ba38ee20a 100644 --- a/designer/server/src/routes/admin/index.js +++ b/designer/server/src/routes/admin/index.js @@ -150,11 +150,7 @@ async function processForm(metadata, token, archive) { archive.append(metadataJson, { name: metadataName }) // Then append definitions under {id}/definition_*.json - const definition = await getFormDefinitions( - metadata.id, - !!metadata.draft, - token - ) + const definition = await getFormDefinitions(metadata.id, token) appendFormDefinitionsToArchive(metadata.id, archive, definition) } @@ -322,16 +318,13 @@ function appendManifestToArchive(archive, totalForms, manifestEntities) { /** * Retrieve both live and draft form definitions * @param {string} id - The form metadata ID - * @param { boolean } includeDraft - Whether to include draft definition * @param {string} token - Auth token - * @returns {Promise<{ liveDefinition: FormDefinition | null, draftDefinition: FormDefinition | null }>} + * @returns {Promise<{ liveDefinition?: FormDefinition , draftDefinition?: FormDefinition }>} */ -async function getFormDefinitions(id, includeDraft, token) { +async function getFormDefinitions(id, token) { const [liveDefinition, draftDefinition] = await Promise.all([ forms.getLiveFormDefinition(id, token).catch(ignore404Error), - includeDraft - ? forms.getDraftFormDefinition(id, token).catch(ignore404Error) - : Promise.resolve(null) + forms.getDraftFormDefinition(id, token).catch(ignore404Error) ]) return { liveDefinition, draftDefinition } @@ -339,12 +332,12 @@ async function getFormDefinitions(id, includeDraft, token) { /** * - * @param {Error & {statusCode: StatusCodes}} err - * @returns {null | never} + * @param {Error & {data?:{statusCode: StatusCodes}}} err + * @returns {undefined | never} */ function ignore404Error(err) { - if (err.statusCode === StatusCodes.NOT_FOUND) { - return null + if (err.data?.statusCode === StatusCodes.NOT_FOUND) { + return undefined } throw err } @@ -370,5 +363,5 @@ function appendFormDefinitionsToArchive(id, archive, definition) { /** * @import { ServerRoute , Request, ResponseToolkit, ResponseObject} from '@hapi/hapi' - * @import { FormMetadata, FormDefinition ,FormMetadataState} from '@defra/forms-model' + * @import { FormMetadata, FormDefinition } from '@defra/forms-model' */ diff --git a/designer/server/src/routes/admin/index.test.js b/designer/server/src/routes/admin/index.test.js index 5e0a8b7b3..6cd143beb 100644 --- a/designer/server/src/routes/admin/index.test.js +++ b/designer/server/src/routes/admin/index.test.js @@ -368,10 +368,18 @@ describe('System admin routes', () => { yield formWithoutDraft }) + const notFoundError = Object.assign(new Error('Not found'), { + data: { statusCode: StatusCodes.NOT_FOUND } + }) + jest .mocked(forms.getLiveFormDefinition) .mockResolvedValue(testFormDefinitionWithSinglePage) + jest + .mocked(forms.getDraftFormDefinition) + .mockRejectedValueOnce(notFoundError) + const options = { method: 'post', url: '/admin/index', @@ -383,10 +391,10 @@ describe('System admin routes', () => { expect(response.statusCode).toBe(StatusCodes.OK) - // Verify only live definition was requested (draft wasn't fetched because metadata.draft is undefined) + // Verify live definition was requested expect(forms.getLiveFormDefinition).toHaveBeenCalledTimes(1) - // Draft definition should not be called when form.draft is undefined - expect(forms.getDraftFormDefinition).not.toHaveBeenCalled() + // Draft definition is always requested and 404 is ignored + expect(forms.getDraftFormDefinition).toHaveBeenCalledTimes(1) // Verify audit event published expect(publishFormsBackupRequestedEvent).toHaveBeenCalledWith( @@ -437,7 +445,7 @@ describe('System admin routes', () => { // Verify definitions were requested expect(publishFormsBackupRequestedEvent).not.toHaveBeenCalled() expect(forms.getLiveFormDefinition).toHaveBeenCalledTimes(2) - expect(forms.getDraftFormDefinition).toHaveBeenCalledTimes(1) + expect(forms.getDraftFormDefinition).toHaveBeenCalledTimes(2) }) test('should ignore 404 when fetching live definition', async () => { @@ -448,7 +456,7 @@ describe('System admin routes', () => { }) const notFoundError = Object.assign(new Error('Not found'), { - statusCode: StatusCodes.NOT_FOUND + data: { statusCode: StatusCodes.NOT_FOUND } }) jest @@ -486,7 +494,7 @@ describe('System admin routes', () => { }) const notFoundError = Object.assign(new Error('Not found'), { - statusCode: StatusCodes.NOT_FOUND + data: { statusCode: StatusCodes.NOT_FOUND } }) jest