balderdashy / waterline-docs

WARNING: The content in this repo is out of date! See https://github.com/balderdashy/sails-docs for the most up-to-date documentation
452 stars 161 forks source link

email: type or validation ? #100

Closed sylvainlap closed 8 years ago

sylvainlap commented 9 years ago

Hello,

Here: https://github.com/balderdashy/waterline-docs/blob/master/models/data-types-attributes.md it is not said that email is also a type. "email" is only specified here: https://github.com/balderdashy/waterline-docs/blob/master/models/validations.md as a validator. However, email can be a type (should be added in the doc), and the validator for email is always false (see here: https://github.com/sailsjs/anchor/issues/94).

Regards.

devinivy commented 8 years ago

"Email" should be used as a validator and not a type. Where did you find it indicated that email may be used as a type? If there's an issue using email as a validator, then that should be addressed as a bug.

jeff-kilbride commented 8 years ago

The email stuff is confusing. You have 'email' used as a type in the last two examples on this page:

http://sailsjs.org/documentation/concepts/models-and-orm/attributes

and listed as a validation rule here:

http://sailsjs.org/documentation/concepts/models-and-orm/validations

So, I've seen both type: "string", email: true and type: "email" in the docs. Honestly, a lot of the validation stuff is confusing. On the validations page above, you have an example with minLength and maxLength. In the list of validation rules, you have minLength, maxLength, and len. And in the code in waterline/lib/waterline/core/validations.js at line 39, you have an example with:

 * attributes: {
 *   name: {
 *     type: 'string',
 *     length: { min: 2, max: 5 }
 *   }
 *   email: {
 *     type: 'string',
 *     required: true
 *   }
 * }

So, I'm not sure if I'm supposed to use minLength, maxLength, len, or length -- or if any/all of them will work. Also, the email validator is definitely not working as reported here:

https://github.com/sailsjs/anchor/issues/94

But that issue has been closed. I am having the same issue with all email addresses I try:

{
  "error": "E_VALIDATION",
  "status": 400,
  "summary": "1 attribute is invalid",
  "model": "Customers",
  "invalidAttributes": {
    "email": [
      {
        "rule": "email",
        "message": "\"email\" validation rule failed for input: 'joe@gmail.com'"
      }
    ]
  }
}

I will try to test just Validator.js and see if I get the same kind of error with all email addresses.

devinivy commented 8 years ago

Sails.js uses abstractions on top of waterline, the specifics of which I'm not familiar with– it does some interpreting of model definitions that waterline on it own does not do. I'd suggest keeping to the waterline-docs repo for documentation of waterline used outside of sailsjs. In general, validations are ruled by anchor. So see here for supported waterline validations: https://github.com/sailsjs/anchor/blob/master/lib/match/rules.js. Regrettably there probably are not docs up to date with all the available anchor validations. Would take a PR.

jeff-kilbride commented 8 years ago

Ok. Since I have your ear... :) I tried with Validator.js and the isEmail() function is working properly. From a directory where I had sails installed, I did this:

var validator = require('./node_modules/sails/node_modules/anchor/node_modules/validator');
console.log(validator.isEmail('joe@gmail.com'));

and got true on the console. So, the email issue is definitely not a Validator.js issue. Where should I report this? At sails or anchor? I'd like to test anchor with the same address, but the docs are very thin and I'm not sure what anchor is expecting.

devinivy commented 8 years ago

Perhaps write a failing test for anchor. This might help: https://github.com/sailsjs/anchor/blob/e7fbec2a9c94cbbd4b68a504a439b90d2472aae8/test/basicType.test.js#L58-L64. It's possible that the options argument (2nd argument) to Validate.isEmail doesn't match-up with arguments that anchor/waterline is passing along to the function.

jeff-kilbride commented 8 years ago

Yep. That's exactly what is happening. The 2nd argument being passed is a boolean primitive, rather than an object, and validator is blowing up on that. I've added a note with the correct stack trace to this issue:

https://github.com/sailsjs/anchor/issues/94

particlebanana commented 8 years ago

This should have been patched in Anchor 0.11.0. Let me know if thats not the case.