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

Example wrong for beforeValidate #70

Open eddieajau opened 9 years ago

eddieajau commented 9 years ago

The example for beforeValidate assumes that Probable_suspects is exposed globally, as it would be in Sails. However, this is not the case for Waterline, but it does lead me to ask can we expose some API to be able to get other collections whilst in the life-cycle hooks.

The immediate problem is how to correct the docs. I guess this will require a note to say that the application developer needed to expose a global for the model in the bootstrap code (I actually dislike polluting the global namespace in this way).

And/or we can solve it by deciding that the example use-case is useful and it need to lead to a code fix?

For example, something like this would be ideal:

  beforeValidate: function(citizen_record, next){
    var Probable_suspects = this.getCollection('Probable_suspects');

    Probable_suspects.findOne(citizen_record.citizen_id).exec(function(err, suspect) {
      if(err) return next(err);
      if(!suspect) return next(new Error('This citizen is not a suspect'));
      next();
    });
  }

Thanks in advance.

devinivy commented 9 years ago

I achieve this in my applications by passing the waterline instance into the scope of the model definition.

// Model.js
module.exports = function(waterline) {

     return {/* model definition */};
}

See https://github.com/devinivy/dogwater/blob/master/lib/index.js#L17-L35

I do agree that a public API for this would be useful, as it's a common issue that gets asked!

dmarcelino commented 9 years ago

I want to start a new helper project to make waterline more user friendly when used standalone but still didn't get around to do it...

I too don't like polluting the global namespace, my strategy is to add an attribute instance to all my model definitions containing the model instance. Then, in my controller I can do:

var User = require('user').instance;

I however usually avoid using models in my lifecycle callbacks as I don't like having CRUD code in my model definition files. For cases like yours I would probably create some sort of ControllerUtil class to do that. Just a design preference, I guess... Nothing I feel too strong about.

dmarcelino commented 9 years ago

BTW @eddieajau, have you tried using this, apparently added in balderdashy/waterline#717:

beforeValidate: function(values, cb) {
  this.findOne({
    where: criteria,
    sort: { updatedAt: 'desc' }
  }).exec(function(err, user) {
    values.counter = user ? 0 : user.counter + 1;
    cb();
  });
eddieajau commented 9 years ago

Another example of why we need a code+docs contribution policy :)

I agree there are lots of strategies for solving the example, but I think the immediate problem is the example needs explain the assumptions about where the dependency might have come from. Probably just a sentence explaining that Probable_suspects is in the scope of the code "somehow"?

dmarcelino commented 9 years ago

Another example of why we need a code+docs contribution policy :)

I agree it's good people provide docs but I don't like being too strict about it as it may make some people lose interest in contributing if they feel it's an involved process.

the example needs explain the assumptions about where the dependency might have come from

Agreed.