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

Validate on runs validation on all attributes #124

Closed
kat3kasper opened this issue Feb 18, 2016 · 7 comments
Closed

Validate on runs validation on all attributes #124

kat3kasper opened this issue Feb 18, 2016 · 7 comments

Comments

@kat3kasper
Copy link

I'm wondering why when using on during validate, validate still validates each attr and only stores the results of those whitelisted. This seems like unnecessary processing. Why not only run the validation if on is not defined or the attr is specified.

function validate(options = {}, async = true) {
  var model = get(this, 'model');
  var whiteList = makeArray(options.on);
  var blackList = makeArray(options.excludes);
  var validationResult, value;

  var validationResults = get(this, '_validatableAttributes').reduce((v, name) => {

    if (!isEmpty(blackList) && blackList.indexOf(name) !== -1) {
      return v;
    }

    if (isEmpty(whiteList) || whiteList.indexOf(name) !== -1) {
      validationResult = get(this, `attrs.${name}`);

     // If an async validation is found, throw an error
     if (!async && get(validationResult, 'isAsync')) {
       throw new Error(`[ember-cp-validations] Synchronous validation failed due to ${name} being an async validation.`);
     }

      value = get(validationResult, 'value');
      v.push(validationResult);
    }
    return v;
  }, []);

  var validationResultsCollection = ValidationResultCollection.create({
    content: validationResults
  });

  var resultObject = {
    model,
    validations: validationResultsCollection
  };

  if (async) {
    if (get(validationResultsCollection, 'isAsync')) {
      resultObject.promise = get(validationResultsCollection, 'value');
    }
    return RSVP.hash(resultObject);
  }

  return resultObject;
}
@offirgolan
Copy link
Collaborator

Im not sure I'm understanding you question but if on is provided, we only validate the attributes that are given in that collection

 if (isEmpty(whiteList) || whiteList.indexOf(name) !== -1) { ... }

@kat3kasper
Copy link
Author

That is the thing we don't only validate the attributes that are in the given collection. We validate all the attributes but only return the results of the whitelisted attributes.

The original validate calls validationResult = get(this, 'attrs.${name}'); on all attributes then if whitelisted adds the results to validationResults.

My updated code above only runs the validations if the attribute is whitelisted.

@offirgolan
Copy link
Collaborator

Oh you posted modified code! I thought that was the original code lol.

I see what you mean! I'll make the change 😸

@offirgolan
Copy link
Collaborator

Thanks for creating the issue and bringing this to my attention!

@kat3kasper
Copy link
Author

If you add this to the CHANGELOG can you attribute it to me? thank you

@stefanpenner
Copy link
Collaborator

good catch

@offirgolan
Copy link
Collaborator

Fixed with v2.4.1, attributed you in the changelog as well 😄

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

3 participants