Skip to content

Bugfix/handle errors in file upload 2 #280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mokhld wants to merge 1 commit into from
Closed

Bugfix/handle errors in file upload 2 #280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/server/plugins/engine/pageControllers/SummaryPageController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
type SubmitPayload
} from '@defra/forms-model'
import Boom from '@hapi/boom'
import { type RouteOptions } from '@hapi/hapi'
import { type ResponseToolkit, type RouteOptions } from '@hapi/hapi'

import { COMPONENT_STATE_ERROR } from '~/src/server/constants.js'
import { ComponentCollection } from '~/src/server/plugins/engine/components/ComponentCollection.js'
Expand Down Expand Up @@ -162,6 +162,7 @@ export class SummaryPageController extends QuestionPageController {
)

request.yar.flash(COMPONENT_STATE_ERROR, govukError, true)
await request.yar.commit(h as ResponseToolkit)
Copy link
Copy Markdown
Contributor

@alexluckett alexluckett Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duh, don't know what I wanted to put this into context.js 🤦

thanks!

Copy link
Copy Markdown
Contributor

@alexluckett alexluckett Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remembered now. It's because yar needs to commit the internal clear operation when yar.flash(key) is called. Otherwise the error will always show and never disappear.


await cacheService.resetComponentStates(request, error.getStateKeys())

Expand Down
22 changes: 7 additions & 15 deletions src/server/plugins/nunjucks/context.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,12 @@ export async function context(request) {
consumerViewContext = await pluginStorage.viewContext(request)
}

const flashedError = request.yar.flash(COMPONENT_STATE_ERROR)
const flashedErrors = !Array.isArray(flashedError)
? [flashedError]
: undefined
const flashResult = /** @type {unknown} */ (
request.yar.flash(COMPONENT_STATE_ERROR)
)
const flashedErrors = /** @type {object[]} */ (
Array.isArray(flashResult) ? flashResult : [flashResult].filter(Boolean)
)

/** @type {ViewContext} */
const ctx = {
Expand All @@ -62,19 +64,9 @@ export async function context(request) {
previewMode: isPreviewMode ? formState : undefined,
slug: isResponseOK ? params?.slug : undefined,
// @ts-expect-error TODO fix and merge into 'errors' to link back to the form component
flashedErrors
flashedErrors: flashedErrors.length ? flashedErrors : undefined
}

// TODO remove this temporary hack
await request.yar.commit(
/** @type {import('@hapi/hapi').ResponseToolkit} */ (
/** @type {unknown} */ ({
// eslint-disable-next-line @typescript-eslint/no-empty-function
state: () => {}
})
)
)

return ctx
}

Expand Down
5 changes: 4 additions & 1 deletion test/form/component-state-errors.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,9 @@ describe('Component State Error Tests - File Upload', () => {
expect(submitRes.statusCode).toBe(StatusCodes.SEE_OTHER)
expect(submitRes.headers.location).toBe(`${basePath}/file-upload-component`)

// Get updated session cookie after the flash was committed
const updatedHeaders = getCookieHeader(submitRes, 'session')

// Now GET the file upload page and verify the error message is displayed
jest.mocked(uploadService.initiateUpload).mockResolvedValueOnce({
uploadId: '123-546-791',
Expand All @@ -419,7 +422,7 @@ describe('Component State Error Tests - File Upload', () => {

const pageRes = await server.inject({
url: `${basePath}/file-upload-component`,
headers
headers: updatedHeaders
})

expect(pageRes.statusCode).toBe(StatusCodes.OK)
Expand Down
Loading