longshotlabs / simpl-schema

A JavaScript schema validation package that supports direct validation of MongoDB update modifier objects
https://www.npmjs.com/package/simpl-schema
MIT License
560 stars 115 forks source link

multiple validation errors with same key/name are not shown & some notes on the docs #456

Open niklasdahlheimer opened 2 years ago

niklasdahlheimer commented 2 years ago

I do password validation in the custom property. I.e.

const userSchema = new SimpleSchema({
    "_id": {type: String, optional: true},
    "password": {
        type: String,
        optional: true,
        custom: function () {
            // if this is an update ("_id" already set) and password is not set -> okay
            if (this.field("_id")?.isSet && !this.isSet) return undefined;
            // if this is an insert ("_id" NOT set, will be set by collection) and password is not set -> error
            if (!this.field("_id")?.isSet && !this.isSet) return SimpleSchema.ErrorTypes.REQUIRED;

            const validationResult = ...  // password validation logic...

            // no validation errors -> password is valid
            if (validationResult.length == null) return undefined;

            // if not valid (validationResult array has entries), add validation errors to validation context
            this.addValidationErrors([
               // !!!! only one of these error will be shown because they have the same name property
               {name: this.key, type: "minPasswordLengthErrorType", value. this.value, count: MIN_PASSWORD_LENGTH},
               {name: this.key, type: "minDigitsCountErrorType", value. this.value, count: MIN_DIGITS_COUNT},
               /// [...]
            ]);

            // should return false if validation errors had been added manually to the validation context
            return false;
        }
    },
// [...]
});

A few things about this:

  1. Looks like it's not possible to add muliple ValidationErrors with the same name property. Only one ValidationError will be shown. So name can not be used for the schema key (as recommended the docs) if there are multiple errors for one key (which usually is the case for a password which should meet multiple requirements)
  2. The docs could mention (also here) that it's possible to pass more custom fields via the validationError-objects, like I did with count and like it's probably done with SimpleSchema.ErrorTypes.MIN_NUMBER which uses {{min}} in the messageBox
  3. The docs lacks of an one example with a function as property. Especially for autoValue I was struggeling at beginning. Maybe you could add something like this to the autoValue section. And maybe a note that arrow functions will NOT work in the function properties section

const schema = new SimpleSchema({ firstName: String, lastName: String, email: { type: String, optional: true, regEx: SimpleSchema.RegEx.EmailWithTLD,
}, nameWithEMail: { type: String, autoValue: function(){ const fullName = ${this.field('firstName').value} ${this.field('lastName').value} const emailAppend = this.field('email').isSet ? (${this.field('email').value}) : "" return fullName.concat(emailAppend); } } })



I could create a pull request for these doc-notes if you wish.
Great package btw :)
Greetings
github-actions[bot] commented 2 years ago

Thank you for submitting an issue!

If this is a bug report, please be sure to include, at minimum, example code showing a small schema and any necessary calls with all their arguments, which will reproduce the issue. Even better, you can link to a saved online code editor example, where anyone can immediately run the code and see the issue.

If you are requesting a feature, include a code example of how you imagine it working if it were implemented.

If you need to edit your issue description, click the [...] and choose Edit.

Be patient. This is a free and freely licensed package that I maintain in my spare time. You may get a response in a day, but it could also take a month. If you benefit from this package and would like to see more of my time devoted to it, you can help by sponsoring.

aldeed commented 9 months ago

Since the beginning this package has only included the first error per field. It was a bit of an intentional choice, making various things simpler. It just means that the user will fix the errors one by one, but eventually they do see them all.

I can understand why that might not meet the needs of some situations, but I worry about changing that behavior that people likely rely on. So I think someone could submit a PR adding support for multiple errors per name, but it should be something you have to opt in to with a new validation option (e.g., multipleErrorsPerKey: true)