refactor: use combined route validators and handlers #2819
Conversation
|
Only added the change for the contact verification OTP for now, to verify the approach and also to check if there are any particular routes to not put this change on. |
There was a problem hiding this comment.
thank you @tehtea for the contribution, the approach is correct! in addition, you can delete the validation middleware for this route from user.routes.ts so that the middleware does not run twice.
| * Celebrate validation for the contact OTP verification endpoint. | ||
| */ | ||
| export const validateContactOtpVerificationParams = celebrate({ | ||
| body: Joi.object({ |
There was a problem hiding this comment.
we normally use the Segments enum exposed by celebrate to avoid accidentally misnaming the keys. see user.routes.ts for reference
|
Done! Nah, thank you all for open-sourcing this project, I learned a lot of good practices from here! Applied the changes to other routes I could find, feel free to take a look. |
karrui
changed the title
|
Some checks seem to not be starting, @tehtea do you mind pushing an empty commit to see if that would retrigger the CI/CD? |
|
Sure, let me give it a try
…
On 12 Oct 2021, at 8:41 PM, Kar Rui Lau ***@***.***> wrote:
Some checks seem to not be starting, @tehtea do you mind pushing an empty commit to see if that would retrigger the CI/CD?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
|
|
@tehtea sorry for the slowness, still figuring out why only some CI/CD are running... :( |
|
@karrui Hey no worries! Just a minor refactor after all, not sure how to trigger the test actions on my end too... |
|
@tehtea can you try merging develop into this PR and see if the CI/CD gets triggered? If that still does not work, as a last resort, I will pull your branch and reopen this PR |
…ntact verification OTP
…on and use Segments enum
tehtea
force-pushed
the
refactor/combine-validators-and-handlers
branch
from
d7cdbf5 to
43af205
Compare
|
@karrui Okay, just rebased the commits and force-pushed, let's see how it goes |
|
Sorry for the wait, we should be able to trigger our CI/CD once #3175 is merged in. |
|
@tehtea should work now if you rebase and push again. Thank you so much for being patient and we're so sorry that it took so long to get this handled :( |
|
@karrui No worries, these things happen :) pushed some minor changes, here's hoping it works. |
|
merged in #3428 |
Problem
Prevent developers from forgetting the use of validators when using a controller handler.
Closes #1642
Solution
As suggested in the issue, put the original handler with a validation middleware in an array, and expose the final array for usage rather than the original handler
Breaking Changes
Improvements:
Tests
Ran a regression test using
npm run test-backend, and ran happy-path testing locally with the changed routes