jedireza / aqua

:bulb: A website and user system starter
https://jedireza.github.io/aqua/
MIT License
1.38k stars 356 forks source link

Centralize validation #214

Closed mjp0 closed 7 years ago

mjp0 commented 7 years ago

Not an issue but a request/enhancement: Please centralize model validation.

For example, I need to allow username to be something more than a token and there's files all over the place where the field is validated with Joi and all validations are hardcoded. Now I need to change every single file and then trial/error those validations that I didn't remember. This type of code is unnecessary and makes Aqua PITA to modify.

jedireza commented 7 years ago

Yeah there is model validation and there's api input validation. Unfortunately they're not the same and I'm not sure if they could be in all cases.

We don't use model validation anywhere in Aqua or Frame, but it is built-in to mongo-models: https://github.com/jedireza/mongo-models/blob/master/API.md#validatecallback

If the username's aren't validated on the way into the API with Joi, you're probably stuck with validating them manually in a pre-handler. Could you share more about the username/token change you're working on?

mjp0 commented 7 years ago

I understand what you mean and having different validation models for API and internal is totally ok, if they are in same files. And please understand that in my opinion Aqua's code is great and it's a pleasure to work with. This enhancement is a bit like complaining that the dashboard material is a bit plasticky in Tesla :)

What I mean is that there's three separate username: Joi.string().token().lowercase().required() validations hardcoded in https://github.com/jedireza/aqua/blob/master/server/api/users.js - I don't see why these couldn't be validated from one file containing validations for common fields like username.

My case was really simple and I got it working easily. I just needed to get rid of token() in the validation and I was able to push email as a username in the signup form. I wanted to simplify the signup flow by removing username altogether and using email as the username.

jedireza commented 7 years ago

😁 I see. Thanks for sharing this. I don't think I'll take action on this right away, but it won't be forgotten.