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

Add a whereIf method to query builder #17

Open
rohit-gohri opened this issue Feb 13, 2019 · 4 comments
Open

Add a whereIf method to query builder #17

rohit-gohri opened this issue Feb 13, 2019 · 4 comments

Comments

@rohit-gohri
Copy link
Contributor

Something like:

Model.query().whereIf(condition, 'id', options.id);
@hit12369
Copy link
Member

hit12369 commented Feb 15, 2019

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
Copy link
Contributor Author

// 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 rohit-gohri assigned rohit-gohri and unassigned rajat-smpx May 8, 2019
@rohit-gohri
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@rohit-gohri rohit-gohri removed their assignment Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants