fix(encrypt-submission): shift encrypt mode form guard higher up the pipeline #1929
Conversation
There was a problem hiding this comment.
actually might it be better to use EncryptSubmissionService.checkFormIsEncryptMode, and use mapRouteError to get the status code and error message?
advantages are
- a step towards keeping all the status codes and error messages in one place
- easier to chain
Results in future if someone wants to clean up this controller, like @seaerchin did forsubmitEmailModeForm
And move higher up the pipeline so that the `form` variable has the new type.
|
good idea, i just switched the approach and moved it higher up the pipeline so that the |
| const checkFormIsEncryptModeResult = checkFormIsEncryptMode(formResult.value) | ||
| if (checkFormIsEncryptModeResult.isErr()) { | ||
| logger.error({ | ||
| message: | ||
| 'Trying to submit non-encrypt mode submission on encrypt-form submission endpoint', | ||
| meta: logMeta, | ||
| }) | ||
| const { statusCode, errorMessage } = mapRouteError( | ||
| checkFormIsEncryptModeResult.error, | ||
| ) | ||
| return res.status(statusCode).json({ | ||
| message: errorMessage, | ||
| }) | ||
| } | ||
| const form = checkFormIsEncryptModeResult.value |
There was a problem hiding this comment.
actually i think neverthrow solves this problem of repeated error checking - we can just chain this into the next method and be confident that the form is already encrypted!
then, the mapErr will be applied to the result of the whole expression (inclusive) of this! we could do a pass through on the err (using mapErr) so that the type is preserved but logs are still present too.
idk if it's beyond the scope of this PR but yea, just something to think about, especially because lines 70 - 186 is essentially just checking the form
There was a problem hiding this comment.
thanks for the review! yup the end goal will be definitely to refactor the whole pipeline to use the same map/mapErr pattern as the email-submission endpoint. but that should be left for later after all the upcoming work on the encrypt-submission pipeline is completed
Problem
The guard to prevent submisison of non-storage mode forms in storage mode submission endpoint is located very low in the pipeline, when it should have been one of the first checks made.
Reordering this above will also help reflect the typing of the
formobject accurately, earlier in the pipeline.Solution
Copy paste, with changes in wording of the log messaging