refactor: move logic for validateEncryptSubmission to utility #627
Conversation
|
@karrui thanks for discussing earlier today, could I trouble you to take a look to see if this approach is roughly correct? :) will clean up / write tests and work on submissions thereafter since you are refactoring the rest of encrypt-submissions |
tshuli
force-pushed
the
refactor/ts-encrypt-submissions
branch
from
d468602 to
e4e2298
Compare
There was a problem hiding this comment.
I think the things you've refactored out as utilities actually belong in the domain of the controller and does not need to be extracted.
I think what you can do is to convert the utility functions currently being used into neverthrown functions so you don't need to wrap them in a try catch block.
|
|
||
| type ReqWithForm<T> = T & { form: IPopulatedForm } | ||
|
|
||
| export const isValidEncryptSubmission = (req: Request): boolean => { |
There was a problem hiding this comment.
I don't think we need to create the isValidEncryptSubmission function, can't we use the checkIsEncryptedEncoding function directly?
I think we can update the checkIsEncryptedEncoding function into a neverthrown function instead.
|
|
||
| type ReqWithForm<T> = T & { form: IPopulatedForm } | ||
|
|
||
| export const isValidEncryptSubmission = (req: Request): boolean => { |
There was a problem hiding this comment.
Utility functions should also not need to be aware of other layers, in this case this function needs to know about req from the controller layer, which is not what we want
| return true | ||
| } | ||
|
|
||
| export const processResponses = (req: Request): void => { |
There was a problem hiding this comment.
Same here. The argument for this function should not be Request, but scoped to the variable it is going to act on. In this case, this whole block seems to be the domain of the controller and does not need to be extracted.
Also strive to make utility functions pure. In this function, we are assigning to req.body.parsedResponses as a side effect, which can be hard to realize
|
@karrui Thanks a lot! |
No description provided.