leepowelldev / mongoose-validator

Validators for mongoose models utilising validator.js
MIT License
378 stars 43 forks source link

making mongoose-validator compatible with node-validator 3.x #15

Closed igorescobar closed 10 years ago

igorescobar commented 10 years ago

Hi,

I did some small changes in order to make mongoose-validator compatible with node-validator 3.x.

Hope you enjoy it.

leepowelldev commented 10 years ago

Thanks @igorescobar - I'm going to close this pull request. Not because it's not worthy, but because I'd already made a start with a new release. I'd also added some new tests on which this was failing - so I've merged both of ours together. I'll release a 3.0.0_beta branch soon for people to test with.

igorescobar commented 10 years ago

Let me know if you need help with something.

Sent from my iPhone

On 24/06/2014, at 05:11, leepowellcouk notifications@github.com wrote:

Thanks @igorescobar - I'm going to close this pull request. Not because it's not worthy, but because I'd already made a start with a new release. I'd also added some new tests on which this was failing - so I've merged both of ours together. I'll release a 3.0.0_beta branch soon for people to test with.

— Reply to this email directly or view it on GitHub.

igorescobar commented 10 years ago

You're not going to keep retro compatibility with defaultError present in 1.x and 2.x?

leepowelldev commented 10 years ago

I'm not particularly keen on tying this to the exact available validators validator.js provides - as we've seen these are prone to changing (between 2.0 and 3.0).

Version 3.0 of validator.js is quite a large change, a number of validators have either been removed or renamed so 0.3.0 of mongoose-validator isn't going to be a straight drop-in update.

igorescobar commented 10 years ago

Yep. In my PR I did the necessary changes in the object to make sure that it's compatible with node-validator 3.x.

About the new version of mongoose-validator it should be 1.0.0 acording to http://semver.org/. This is going to have major changes and it will break previous users so it should be a major release not just a MINOR increment.

Take a look into https://github.com/kawamanza/step-up it will take care of those things for you.

Let me know if you need any help with it.

leepowelldev commented 10 years ago

You're right, it should be a 1.0.0 release - I'll take care of that.

I'll take another look at your PR and reconsider adding more verbose default error messages for the available validators.

igorescobar commented 10 years ago

Sure, I think it should be reconsidered in order to make this upgrade more "easy" to previous users.

There is a lot of changes since 1.x and would be nice to introduce this upgrade to all users without too much pain.

I think you should release two new versions. A 0.3.0 with retro compatibility and the node-validator 3.x support and a v1.0.0 with no retro-compatibility at all... just to make sure that the users will have options to make this upgrade more easy.

What do you think?

leepowelldev commented 10 years ago

As there are so many changes in validator.js 3.x I'm tempted to leave the the 0.2.x version of mongoose-validator, and then release a 1.0.0 with only 3.x support. Releasing a 0.3.0 version with retro capability will not really give users any additional functionality - and is more likely to break their projects and cause issues.

igorescobar commented 10 years ago

Yeah... you're maybe right. Better stick with the v1.0.0 of mongoose-validator. Any prevision on the release date?

leepowelldev commented 10 years ago

Will do a bit more testing on it tonight, and then hopefully release. I've re-added your default error messages :)

igorescobar commented 10 years ago

Wooot! Nice! I'm going to use it in my project ;o)

leepowelldev commented 10 years ago

Cool! Let me know if you come across any problems... would be good having someone using it.

igorescobar commented 10 years ago

Sure! Will do!

leepowelldev commented 10 years ago

:thumbsup:

leepowelldev commented 10 years ago

Hmm, now we're considering a full non-retro release - I'm now considering changing how validators are created by the user to passing just one options object:

var mongooseValidator = require('mongoose-validator');
var validator = mongooseValidator.validate({
  method: 'isLength',
  message: 'Invalid length',
  passIfEmpty: true,
  arguments: [4, 40]
});

What do you think?

igorescobar commented 10 years ago

Argument based implementations is always a pain and it's hard to keep compatibility as the project goes forward... I think it's a good catch and if the user want to make a it shorter they can easily create their own shortcut like:

var mongooseValidator = require('mongoose-validator');
var validate = function (method, msg, passIfEmpty, args) {
  return mongooseValidator.validate({
    method: method,
    message: msg,
    passIfEmpty: passIfEmpty,
    arguments: args
  });
};
igorescobar commented 10 years ago

So? Any luck?

Sent from my iPhone

On 24/06/2014, at 11:50, leepowellcouk notifications@github.com wrote:

Hmm, now we're considering a full non-retro release - I'm now considering changing how validators are created by the user to passing just one options object:

var mongooseValidator = require('mongoose-validator'); var validator = mongooseValidator.validate({ method: 'isLength', message: 'Invalid length', passIfEmpty: true, arguments: [4, 40] }); What do you think?

— Reply to this email directly or view it on GitHub.

leepowelldev commented 10 years ago

Yep, think I'm pretty much there - just need to update my tests and documentation. Should get released this evening.

igorescobar commented 10 years ago

Nice! Next step I'm going to write a blog post about the release and the mongoose-validator itself.

Sent from my iPhone

On 25/06/2014, at 08:05, leepowellcouk notifications@github.com wrote:

Yep, think I'm pretty much there - just need to update my tests and documentation. Should get released this evening.

— Reply to this email directly or view it on GitHub.

leepowelldev commented 10 years ago

:thumbsup:

leepowelldev commented 10 years ago

Would you mind giving the pre-release a once over? Just in case I've missed anything... https://github.com/leepowellcouk/mongoose-validator/tree/1.0.0_pre_release

igorescobar commented 10 years ago

Will do!

Sent from my iPhone

On 25/06/2014, at 12:20, leepowellcouk notifications@github.com wrote:

Would you mind giving the pre-release a once over? Just in case I've missed anything... https://github.com/leepowellcouk/mongoose-validator/tree/1.0.0_pre_release

— Reply to this email directly or view it on GitHub.

igorescobar commented 10 years ago

Superficial recommendations: 1 - At the README.md:

Version 1.0.0 has been refactored to support a simplier interface and also validator.js 3.0.x - 1.0.0 is not backwards compatible with pre 1.0.0 releases and it is advised to remain with 0.2.x releases.

To

Version 1.0.0 has been refactored to support a simpler interface and also validator.js 3.0.x - 1.0.0 is not backwards compatible with pre 1.0.0 releases of *node-validator* so it is advised to remain with 0.2.x of *mongoose-validator* releases.

2 - At the installation instruction

npm install mongoose-validator

To

npm install mongoose-validator --save

3 - At the usage examples use syntax highlight to make it more clear and easy to understand. 4 - Integrate the lib to travis-ci. In my pull request I've added the manifest file. The only thing you need to do it is to add the travis-ci hook to your github project. This will make sure that every pull request will come with the unit tests results report. It will make sure that upcoming contributions aren't breaking anything BEFORE you merge it. 5 - add the travis-ci badge :) 6 - At mongoose-validator.js:79 Replace the code:

module.exports = {
  validate: validate,
  extend: extend,
  defaultErrorMessages: defaultErrorMessages
};

To

module.exports = validate;
module.exports.extend = extend;
module.exports.defaultErrorMessages = defaultErrorMessages;

So you don't need to use the .validate like this:

var validate = require('mongoose-validator').validate;

It is going to be:

var validate = require('mongoose-validator');

Right? 7 - I've tested the code and everything works fine. :+1:

igorescobar commented 10 years ago

I've edited my previous answer in order to make it easier to understand #LOL

igorescobar commented 10 years ago

Also... would be nice to have the installation instruction of the previous release In case they need backwards compatibility also with the link of the README.md of the previous versions in order to make an easy access to the documentations of the previous release.

igorescobar commented 10 years ago

npm install mongoose-validation@0.2.2 --save

igorescobar commented 10 years ago

The link of the doc would be: https://github.com/leepowellcouk/mongoose-validator/blob/0.2.2/README.md

leepowelldev commented 10 years ago

Morning. Thanks for this, these have been integrated (I didn't know about syntax highlighting :+1:) - and I've integrated with Travis. NPM has now been updated and a 1.0.0 tag released.

igorescobar commented 10 years ago

Wooot!

Sent from my iPhone

On 26/06/2014, at 05:41, leepowellcouk notifications@github.com wrote:

Morning. Thanks for this, these have been integrated (I didn't know about syntax highlighting ) - and I've integrated with Travis. NPM has now been updated and a 1.0.0 tag released.

— Reply to this email directly or view it on GitHub.

leepowelldev commented 10 years ago

Hope you enjoy it - looking forward to the blog post :)

igorescobar commented 10 years ago

Hey, take a look into my repo: https://github.com/igorescobar/jQuery-Mask-Plugin

Would be nice for you to put those "views" badges. It will give you a idea of how popular is your repo ;-)