smartprix / xorm

NodeJS ORM based on ObjectionJS with some extra utilities
18 stars 4 forks source link

Add a `whereIf` method to query builder #17

Open rohit-gohri opened 5 years ago

rohit-gohri commented 5 years ago

Something like:

Model.query().whereIf(condition, 'id', options.id);
hit12369 commented 5 years ago

This might be better instead, as most cases will benefit from this.

// better name?
Model.query().whereIgnoreNull('search', args.search)

// or
Model.query().where('search', args.search, {ignoreNull: true});

Above method might create bugs that are harder to spot, eg.

// might create an undefined error
Model.query().whereIf(ctx.country, 'id', ctx.country.id);

or might be harder to read

// harder to see what the condition is
Model.query().whereIf(args.subCategoryId && args.subCategoryId.length, 'Product.subCategoryId', args.subCategoryId)
rohit-gohri commented 5 years ago
// better name?
Model.query().whereIgnoreNull('search', args.search)

// or
Model.query().where('search', args.search, {ignoreNull: true});

These will only be helpful if the condition is the value being compared. What about if the condition is different?

Above method might create bugs that are harder to spot, eg.

// might create an undefined error
Model.query().whereIf(ctx.country, 'id', ctx.country.id);

Same goes for {ignoreNull: true}:

 // might create an undefined error
 Model.query().where('id', ctx.country.id, {ignoreNull: true});
rohit-gohri commented 5 years ago

Something like whereIgnoreNull also isn't useful for functions or raw params:

Model.query().where((q) => {
    q.where('id', options.id).orWhere('id', '<', options.val)
});

Maybe we can implement both, a general purpose whereIf that checks the condition variable and forwards rest to where:

Model.query().whereIf(condition, 'id', options.id);
Model.query().whereIf(condition2, 'id', '<', options.val);
Model.query().whereIf(condition3, (q) => {
    q.where('id', options.id).orWhere('id', '<', options.val)
});

And a whereIgnoreUndefined for simple cases

rohit-gohri commented 5 years ago

The simple cases can already be handled in objection through skipUndefined: https://vincit.github.io/objection.js/api/query-builder/other-methods.html#skipundefined