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

Feature #4527 "skip" option for custom validators #4528

Merged
merged 1 commit into from Sep 17, 2015

Conversation

2 participants
@superclarkk
Contributor

superclarkk commented Sep 16, 2015

Enable .validate(options.skip) to skip specific model/custom validators #4527
Return false to exit iteration early when using _.each and _.forIn , as per lodash docs

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 17, 2015

Concerning BC: i don't think this constitutes a breaking change since i don't believe you would have model validators named the same as attributes.

/cc @janmeier

@@ -108,7 +108,7 @@ InstanceValidator.prototype._builtinValidators = function() {
var validators = [];
Utils._.forIn(this.modelInstance.rawAttributes, function(rawAttribute, field) {
if (self.options.skip.indexOf(field) >= 0) {
return;
return false;

This comment has been minimized.

@mickhansen

mickhansen Sep 17, 2015

Contributor

Is the iteration supposed to be exited early though? skip might match the first field, should still validate all the other fields.

This comment has been minimized.

@superclarkk

superclarkk Sep 17, 2015

Contributor

It was exiting early with just "return", but the lodash docs state that it should be return false. I changed this line just so that it was consistent with my main changes.

This comment has been minimized.

@mickhansen

mickhansen Sep 17, 2015

Contributor

Are you absolutely sure about that? lodash docs says you have to explicitely return false.
If it's currently exiting the loop early with just return that would be a bug.

This comment has been minimized.

@mickhansen

mickhansen Sep 17, 2015

Contributor

https://github.com/lodash/lodash/blob/master/lodash.js#L3352 it tests for explicit false, return undefined would not break loop.

@superclarkk

This comment has been minimized.

Contributor

superclarkk commented Sep 17, 2015

@mickhansen If by "attributes" you mean fields (table columns), then you are correct. An error is thrown if a custom validator (model validator) and an attribute have the same name.

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 17, 2015

@superclarkk i did mean model/table fields, thanks for confirming :)

@superclarkk superclarkk force-pushed the superclarkk:patch-1 branch from 5217ccd to afecd92 Sep 17, 2015

mickhansen added a commit that referenced this pull request Sep 17, 2015

Merge pull request #4528 from superclarkk/patch-1
Feature #4527 "skip" option for custom validators

@mickhansen mickhansen merged commit cfd4da8 into sequelize:master Sep 17, 2015

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

mickhansen added a commit that referenced this pull request Sep 17, 2015

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 17, 2015

Published in 3.9.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment