-
Notifications
You must be signed in to change notification settings - Fork 87
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.
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
Test: publicform integration tests #1572
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests are fine, but check if there is a regression on private form response
src/app/modules/form/public-form/__tests__/public-form.routes.spec.ts
Outdated
Show resolved
Hide resolved
const expectedResponseBody = JSON.parse( | ||
JSON.stringify({ | ||
message: | ||
'If you think this is a mistake, please contact the agency that gave you the form link.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The body response should have 2 more key-values:
formTitle: string
isPageFound: boolean
check if this is a regression
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch. this is a regression. will update tests and the controller
a9445ab
to
74507d6
Compare
74507d6
to
c90cf27
Compare
c90cf27
to
e4790c5
Compare
message: string | ||
} | ||
|
||
export interface PrivateFormErrorDto extends ErrorDto { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it doesn't make a difference but for the record, it wasn't necessary to change this to an interface, since you can use &
to achieve the same thing
// Specialized error response for PrivateFormError. | ||
// This is to maintain backwards compatibility with the middleware implementation | ||
if (error instanceof PrivateFormError) { | ||
return res.status(statusCode).json({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
noticed you fixed a regression here. in future it would be good to separate this kind of change into a different PR which can be reviewed with greater urgency. if you name your PR as you did here, other devs may (rightly) assume (as I did) that this PR only contains tests.
Problem
current public form end point does not have any integration tests and only has unit tests on the controller. this PR adds integration tests for HTTP errors.
Part of #1513
Solution
Added integration tests for
GET /:formId/publicform
endpoint.