Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

checkRequired overrides email validation message. #39

Open
astik opened this issue Aug 13, 2023 · 2 comments
Open

checkRequired overrides email validation message. #39

astik opened this issue Aug 13, 2023 · 2 comments

Comments

@astik
Copy link
Contributor

astik commented Aug 13, 2023

Hi,
Following 2bd35bb and its origin issue #24, there might be some change of behavior in the way error message are defined.

Considering this schema:

const userSchema = mongoose.Schema({
	email: {
		type: mongoose.SchemaTypes.Email,
		required: [true, "error.common.required"],
		unique: true,
		message: "error.common.email",
	},
});

If an email is invalid for example foobar i'm expecting to get error.common.email as error message but i get error.common.required.
This is due to the fact that checkRequired does not only check for the mandatory value but also for the email validation:

Email.prototype.checkRequired = function (val) {
return typeof val === 'string' && validateEmail(val, this.options)
}

From my POV, checkRequired should focus only on requirement, leaving the email validation to the plugin later lifecycle.

Current code for checkRequired is :

mongoose.SchemaTypes.Email.prototype.checkRequired = function (val) {
	return typeof val === 'string' && validateEmail(val, this.options);
};

My current alternative is to override this function by setting back the default mongoose behavior:

mongoose.SchemaTypes.Email.prototype.checkRequired = function (val) {
	return val != null;
};

That way, requirement is handled as before and will throw error.common.required only if value is missing. In case of incorrect email, checkRequired will be true, passing the requirement validation and will fail later in the plugin lifecycle providing error.common.email error as expected.

Beware that my alternative only care about non null value for requirement. It might be interesting to take care of allowBlank options here as well.

@konsumer
Copy link
Owner

konsumer commented Aug 14, 2023

Hmm, I see what you mean, but I am not sure how it should work instead. At least at the time I made this, I think it just had 1 validation function. The code is very simple, and I haven't actually used it (or even mongoose) in several years, so I am bit out of touch with how it should work. Would you be interested in making a PR? The actual code is smaller than a description of the problem, so it seems like it might be just as easy to just change it to work however you think it should. I will accept whatever you think is a reasonable solution, and publish it for npm.

@astik
Copy link
Contributor Author

astik commented Oct 6, 2024

As you can see i've been away from this topic as well from a long time 😢
I'm not working with mongoose either. Not sure i'll be able to come back to this in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants