Comments (18)
@diego3g Yess, of course, that is a great idea. What is your profile in discord ?? If you want to add me, my nickname is gabriellopes#8871
from umbriel.
Hii @diego3g. Sure, now i understand why the tests were failing... and we do have a bug here, but of course we can fix it. Can you explain me better what this validation per field would be ? Will this be useful when applying the validator to Kafka handlers ?
from umbriel.
I was thinking, and this error start happening when we refactored the code to pass the entire request to validator. Because before, we were passing only the fields we expected to be filled { name, email, password, password_confirmation }
, and the userId
put by the middleware wasn't included . It could be fixed by reverting the changes in the controller and de-structuring the fields before pass them to the validator. What do you think ? But tell me more about the validator you proposed...
from umbriel.
I think we should pass exactly what are the required fields instead of iterating through each request field. Like this:
const validator = new ValidatorCompositor([
new RequiredFieldValidator('name'),
new RequiredFieldValidator('email'),
new RequiredFieldValidator('password'),
new CompareFieldsValidator('password', 'password_confirmation'),
])
from umbriel.
Sure, this is a good approach, i can implement this... Just a suggestion, what if we create only one RequiredFieldsValidator
and pass for it an array with all required fields ? Something like this:
const validator = new ValidatorCompositor([
new RequiredFieldValidator(['name', 'email', 'password', 'password_confirmation'])
])
This way we don't need to instantiate one validator per field. Imagine a request with a lot of them 🤣
What do you think ?
from umbriel.
I think it's better to have one validator per field so we can pass additional options for each validator, for example:
new RequiredFieldValidator('name'),
new RequiredFieldValidator('email', { message: 'Email is required' }),
new RequiredFieldValidator('password'),
new CompareFieldsValidator('password', 'password_confirmation'),
Or maybe we could change it to work like a schema validation:
const validator = new ValidatorCompositor({
name: [new RequiredFieldValidator()],
email: [new RequiredFieldValidator({ message: 'Email is required' })],
password: [
RequiredFieldValidator(),
CompareFieldsValidator({ compare_with: 'password_confirmation' })
],
})
from umbriel.
My only worries here in the validation are about deep nested data like complex objects or arrays, how we would validate like the property street inside this object?
const user = {
address: {
street: string;
}
}
Maybe we could nest ValidatorCompositors?
const validator = new ValidatorCompositor({
address: new ValidatorCompositor({
street: new RequiredFieldValidator()
})
})
Just some thoughts so we can improve this feature.
from umbriel.
Ok, we can create one RequiredFieldValidator
per field. But i liked so much the idea of a schema validator. And we could use it to solve the problem of nested fields, doing something like this:
const validator = new ValidatorCompositor({
name: [new RequiredFieldValidator()],
email: [new RequiredFieldValidator({ message: 'Email is required' })],
address: [
new RequiredFieldValidator({
message: 'Email is required',
nested_fields: [ // optional property
{ name: 'street' }
]
})
],
password: [
RequiredFieldValidator(),
CompareFieldsValidator({ compare_with: 'password_confirmation' })
],
...
...
})
In this example, for the RequiredFieldsValidator
we pass an array, with the nested fields, in the constructor. If this array isn't empty, we iterate on it, and for each nested field, we validate if it is empty or not.
But the problem is actually solved when we make possible that each nested field can have others nested fields, and we go iterating throught every one validating if it is empty or not.
In my mind, each nested field would be something like this:
interface NestedField {
name: string
nested_fields?: NestedField[]
}
If we have more than one nested field, we just pass them inside "parent fields":
{
field: [
new RequiredFieldValidator({
message: 'This field is required',
nested_fields: [
{
name: 'field2',
nested_fields: [
{
name: 'field3'
},
]
} ,
...
]
})
],
}
It's complex but works 🤣 I tested.
from umbriel.
I created a script, of course isn't ready yet, but take a look:
export class RequiredFieldValidator implements Validator {
constructor(
private readonly props: { message: string; nested_fields: NestedField[] } // we pass the properties where when we instantiate a new validator
) {}
public validate(obj: any, field: string): Error { // the validator compositor will pass these properties to the method
if (this.isEmpty(obj[field])) return new Error(this.props.message) // first we validate if the field is empty or not
if (this.props.nested_fields) { // if this field has nested fields, them we iterate through each one
return this.iterate(this.props.nested_fields, obj[field])
}
return null
}
private iterate(fields: NestedField[], obj: any): Error {
for (const field of fields) {
if (this.isEmpty(obj[field.name])) return new Error(this.props.message)
else if (field.nested_fields) {
return this.iterate(field.nested_fields, obj[field.name]) // here is the "magic"... if the nested field has others nested fields, we use recursion to iterate through them too
}
}
return null
}
private isEmpty(field: any): boolean { // validation logic
return (
field === null ||
field === undefined ||
(typeof field === 'string' && field.trim() === '')
)
}
}
from umbriel.
It's a little bit complex, but works. Is a possible solution to the problem of nested fields. What dou you think ? If you aprove this, i can implement together to the refactor of the RequiredFieldsValidator
. But if you think isn't a good idea, we can just solve the bug by using your suggestion of creating a validator per field, and think more about it later.
from umbriel.
I think it's somehow too much complex for now. I prefer the first approach I sent before or if we try using a schema validation library like Joi or Yup.
Maybe it would be better now to use one library like that instead of reimplementing the whole wheel.
from umbriel.
Sure, the first approach is good, solve our problem. I'm going to be working in the implementation ✌
from umbriel.
@gabriellopes00 have u tried using one library like a mentioned before? As these libraries are all only JavaScript i think they would not break our domain layer and we can focus on new features instead of investing so much time in validation.
I think we could have something like:
type FieldValidationError = Record<string, string>
class RegisterUserValidator implements Validator {
validate(): Either<FieldValidationError, null> {
// validate using Joi or Yup
// parse validation error messages to FieldValidationError type
}
}
from umbriel.
All right, we can use an external library... I've already used yup and validator.js. But do you think we should create one validator by controller ? or handler. Because if we use yup for example, we need to create a data shape by expected request body, right ?
Is a good approach using a external library, but i think we should create generic validators to reuse them in more than one place.
Don't worry, the validators are only applied in external layers, the domain is completely safe.
from umbriel.
@gabriellopes00 I think that for now we should not care about reusability of validators, we can create one validator per controller and in the future, if we see too many repetitions, we try to refactor it. This way we can move on with the validation and work on other parts of the application. What do you think?
I have some other big challenges in the application that I would be glad to have your help 😊
from umbriel.
@diego3g Sure, perfect... let's create the validators per controller (and handlers). I'm thinking about using either yup or validator.js, what do you think ? Both of them have many features that would be very helpful for us. Then we could use the libraries to validate only the fields by controllers, and we don't need to worry about a generic script to handle nested fields, to compare fields, or anything else. What do you think ?
I would be very happy to be able to help with future changes.
from umbriel.
I like Yup or Joi. That's great for me.
from umbriel.
@gabriellopes00 It would be good if you could reach me on Discord so we can keep our conversation more dynamic, what u think?
from umbriel.
Related Issues (20)
- Get all senders and templates routes
- Schedule message sending
- Handle bounce events HOT 3
- Add orderings to lists
- Save message editable content as well
- Allow to set account settings
- Update message
- Send a test email before sending to mass
- Message public share URL
- Performance issues HOT 9
- Implement the presentation layer
- Select provider based on configuration
- Dependency Dashboard
- UserRepository is updating in message with User Datas.
- Count subscribers of tag route HOT 1
- List all tags route
- Return tag ID on contact details route
- Message sending progress HOT 1
- Messages stats with distinct users HOT 1
- Case insensitive search
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from umbriel.