-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: implement report problem with form submission API path #36
Conversation
6427f25
to
863e57c
Compare
type Location, | ||
validationResult, | ||
type Schema, | ||
} from "express-validator"; |
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.
Selected https://express-validator.github.io/docs for now.
The way it is implemented in our code (middleware) makes it easy to change in the future if we are not happy with it.
Also, even though express-validator
provides ready to use middleware functions, I decided to create a custom one because it is easier to control the error response when the validation fails. Otherwise we would have to create a new middleware function to catch the specific error thrown by the library.
Existing other options:
- https://github.com/simonplend/express-json-validator-middleware (provides middleware function)
- https://github.com/ajv-validator/ajv (would have to be inserted in middleware function)
1c3e6c5
to
1ac0e79
Compare
email: contactEmail, | ||
type: "Problem", | ||
subject: "Problem with GC Forms / Problème avec Formulaires GC", | ||
tags: [tagFromEnvironmentMode(), "Forms_API_Submission"], |
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.
Tags have been discussed with Jimmy from our support team.
2e40c7c
to
6e1ef1a
Compare
src/config.ts
Outdated
// Internal function to load environment variables | ||
|
||
function loadOptionalEnvVar(envVarName: string): string | undefined { | ||
return process.env[envVarName]; | ||
} | ||
|
||
function loadRequiredEnvVar(envVarName: string): string { | ||
if (process.env.NODE_ENV === "test") { | ||
return "TEST_IN_PROGRESS"; |
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.
Add to re implement this condition as the way we mock the configuration variables during Vitest setup has been implemented in a way that will load the config file (importOriginal
) as is before altering its content for testing purpose. Since the code in the config file will be executed it will try to load variables that don't exist in the Github Action environment.
81e7f5d
to
d8c56e8
Compare
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.
Nice work. Since this project is still very young we'll continue to refactor and see what patterns work and what might need changing as we go along.
Summary | Résumé
Linked with:
====
/problem
API path to report problem with form submissionexpress-validator
package to validate the JSON payload required by the new/problem
POST operation